feat(quiz): the cheap signals nobody was reading (#556) by AndresL230 · Pull Request #575 · SaplingLearn/Sapling

AndresL230 · 2026-08-22T10:14:46Z

Closes#556 (four of five — see Scope below). Workstream H4 of epic #537.

Each of these was already sitting in a table the generation path touches, and none of it reached the prompt.

signal	cost
`graph_nodes.times_studied`	zero queries — already in the row `generate_quiz` reads to resolve the concept name and course, just never selected
learning velocity	one bounded read; `graph_service._compute_velocity`imported, not re-derived
unfinished attempts on this concept	one owner-scoped read, reusing #542's derived-status rule
flashcards for this concept	one read, a count — front/back are encrypted (#518) and neither read nor needed

times_studied is passed into the gatherer rather than re-read: a second query would spend a round-trip to learn something we were already told.

Landed behind the measurement, as the issue requires

Every signal records its own dimension on quiz.started, which shares a request_id with the llm_usage row — so "is this worth its tokens?" is answerable with data rather than taste. That was the issue's stated condition for landing them at all.

It runs as a fourth leg of the gather that already resolves grounding, the recently-asked list and exam proximity — best-effort context exactly like those three, so it adds no latency. And it never raises: a signal nobody strictly needs must not be able to fail a generation.

Two distinctions the code keeps deliberately

None (couldn't tell) is not 0 (a fact about the student). Collapsing them would report "never studied" for a broken query.
A zero velocity is not reported, because _compute_velocity returns 0.0 both for "no recent gain" and "not enough data" — claiming stagnation we can't distinguish from silence. A zero times_studiedis reported: that one is a fact, and exactly what a generator should know.

Scope — one item deliberately left out

Tutor recency via messages.graph_update_json is not here. The issue frames all five as "one query or one extra selected column", and that one isn't: messages has no user_id (it's session-scoped), so it needs a sessions → messages join plus JSONB concept-name matching through PostgREST. That's a different size of change with a different cost profile, and bolting it on would have made the F6 numbers for the other four harder to read.

Left out deliberately rather than done badly. Flagged on the issue so it doesn't evaporate.

Verification

Hermetic 2218 passed / 9 skipped, ruff clean, oracles 0 findings, integration 70 passed, Playwright 47 passed (the one failure is #566, fixed by #568).

🤖 Generated with Claude Code

Four of the five blind spots the issue lists. Each was already sitting in a table the generation path touches, and none of it reached the prompt: * `graph_nodes.times_studied` — in the row `generate_quiz` ALREADY reads to resolve the concept name and course, simply never selected. One more column, zero extra queries; passed INTO the gatherer rather than re-read, because a second read would spend a round-trip to learn something we were already told. * learning velocity — `graph_service._compute_velocity` is imported, not re-derived. #557 is what two copies of one number costs. * unfinished attempts on this concept — trivially available since #542 gave attempts a derived status; the same `completed_at IS NULL` rule, not a second definition of "unfinished". * flashcards for this concept — a COUNT. Front/back are encrypted (#518) and are neither read nor needed. Landed BEHIND the F6 measurement, which is the condition the issue sets: every signal records its own prompt dimension on `quiz.started`, which shares a `request_id` with the `llm_usage` row, so "is this worth its tokens?" is answerable with data rather than taste. Runs as a fourth leg of the gather that already resolves grounding, the recently-asked list and exam proximity — best-effort context exactly like those three, so it costs no added latency. Never raises: a signal nobody strictly needs must not be able to fail a generation. Two distinctions the code keeps deliberately: * `None` (could not tell) is not `0` (a fact about the student). Collapsing them would report "never studied" for a broken query. * a zero VELOCITY is not reported, because `_compute_velocity` returns 0.0 both for "no recent gain" and "not enough data" — claiming stagnation we cannot distinguish from silence. A zero `times_studied` IS reported: that one is a fact, and exactly what a generator should know. NOT included: tutor recency via `messages.graph_update_json`. The issue frames all five as "one query or one extra selected column", and that one isn't: `messages` has no `user_id` (it is session-scoped), so it needs a sessions -> messages join plus JSONB concept-name matching through PostgREST. Left out deliberately rather than done badly; noted on the issue. Hermetic 2218 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

cloudflare-workers-and-pages · 2026-08-22T10:14:51Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	frontend-staging	`ee1c91e`	Commit Preview URL Branch Preview URL	Aug 22 2026, 10:23 AM

coderabbitai · 2026-08-22T10:14:58Z

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:19 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 @coderabbitai review or push new commits to the PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f78995c1-90ee-4268-a513-4ab94cab82bd

📥 Commits

Reviewing files that changed from the base of the PR and between b14fc3b and ee1c91e.

📒 Files selected for processing (4)

backend/routes/quiz.py
backend/services/quiz_signals.py
backend/tests/test_event_capture_seams.py
backend/tests/test_quiz_signals.py

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

supabase · 2026-08-22T10:17:40Z

This pull request has been ignored for the connected project ybgqdonkoqftwrmweuyv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

Five findings. The first is the one this module was written to avoid. **The velocity was inflated up to 14x, and disagreed with the Tree.** `_compute_velocity` derives its window from `recent[0]` as the OLDEST recent event — which is why `get_graph` feeds it `created_at.asc`. I read `desc` (correct, so the LIMIT keeps the NEWEST events) and handed the rows straight over, collapsing `days` to `max(1, 0) == 1`. Four events of +0.1 over ten days: 0.04 on the graph screen, 0.4 in the quiz prompt. So I reintroduced the exact "two copies of one number" failure the docstring cites #557 for — not by copying the algorithm, but by breaking its contract through the argument. Rows are reversed now, and a test pins the two against each other rather than against a literal. **The flashcard signal is removed, not fixed.** It matched `flashcards.topic` against a concept name, and nothing ever writes a concept name there: every writer stores `body.topic`, and the only caller (`Study.tsx`) passes `course.course_name`. So it was a permanent zero plus a wasted request-path round-trip — and worse, it would have had the F6 measurement conclude the signal is worthless when it was simply never wired to real data, which defeats the entire point of landing these behind a measurement. It needs a concept<->card mapping first; noted in the module and on the issue. **In-flight now shares #542's TTL.** Checking only `completed_at`/ `abandoned_at` IS NULL is a second, looser definition of "unfinished": `_attempt_status` treats an in-progress row past the 24h TTL as abandoned even before the lazy sweep stamps it, and that sweep runs on the history reads, not on generation. A student who generated three quizzes last week and never opened their history would have been told they had three in flight. **Counts are exact, not saturated.** `len(rows)` under a LIMIT reported the cap as though it were a fact; `select_with_count` gives the true total for the same round-trip. **The H3 comment was orphaned** — the H4 block had been inserted between the exam-proximity rationale and the code it explains. Moved below it. Also value-checked the new dimensions in the `quiz.started` pin, which had been comparing the payload against itself and so asserted presence but never correctness. Hermetic 2219 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

AndresL230 · 2026-08-22T10:26:32Z

Review round — five findings, and the first one is the irony of this PR

I broke _compute_velocity by reusing it. The module docstring cites #557 for why the 14-day computation is imported rather than copied — and then I fed it rows in the opposite order from its only other caller. _compute_velocity derives its window from recent[0] as the oldest recent event, which is why get_graph reads created_at.asc. I read desc (correct, so the LIMIT keeps the newest events) and handed them straight over, collapsing days to max(1, 0) == 1.

Four events of +0.1 over ten days:

surface	velocity
Tree / graph screen	0.04
quiz prompt (mine)	0.4

Same concept, two numbers — the exact failure #557 spent a workstream undoing, reintroduced through the argument instead of the algorithm. Rows are reversed now, and the test pins the two computations against each other rather than against a literal.

The flashcard signal is removed, not fixed. It matched flashcards.topic against a concept name, and nothing ever writes a concept name there: routes/flashcards.py stores body.topic, and the only caller (Study.tsx:711) passes course.course_name. So it was a permanent zero plus a wasted request-path round-trip — and worse, it would have had the F6 measurement conclude the signal is worthless when it was simply never wired to real data. That defeats the entire point of landing these behind a measurement. It needs a concept↔card mapping first; documented in the module and flagged on the issue.

In-flight now shares #542's TTL. Checking only completed_at/abandoned_at IS NULL is a second, looser definition of "unfinished": _attempt_status calls an in-progress row past the 24h TTL abandoned even before the lazy sweep stamps it, and that sweep runs on the history reads — not on generation. A student who generated three quizzes last week and never opened their history would have been told they had three in flight.

Counts are exact, not saturated.len(rows) under a LIMIT reported the cap as though it were a fact about the student; select_with_count gives the true total for the same round-trip.

The H3 comment was orphaned — my H4 block had been inserted between the exam-proximity rationale and the code it explains. Moved below it.

Also value-checked the new dimensions in the quiz.started pin, which the review noted were comparing the payload against itself — asserting presence but never correctness.

Scope, restated

This closes three of the five signals #556 lists (times_studied, velocity, in-flight attempts). Two are deliberately not here, both for the same reason — they'd measure nothing as specified:

flashcard review state — no concept-level data exists to read (above);
tutor recency — messages has no user_id, so it needs a sessions → messages join plus JSONB concept matching, which is not "one query" like the others.

Verification

Hermetic 2219 passed / 9 skipped, ruff clean, oracles 0 findings, integration 70 passed, Playwright 47 passed (the one failure is #566, fixed by #568). CI green.

…pports (#556) (#592) * feat(quiz): land the two deferred H4 signals at the scope the data supports (#556) #575 shipped three of the five H4 personalization signals and deferred the other two because, as the issue specified them, both would have measured nothing: * flashcards have no concept link — `topic` is free text every writer sets to the COURSE name — so a concept-name match is a permanent zero; * tutor recency needs a sessions -> messages join (`messages` has no `user_id`) plus JSONB concept matching. Both now land: the first at COURSE scope rather than pretending to concept scope, the second as one bounded owner-scoped `sessions` read plus one `messages` read over the ids it returns. Five new fields, five new F6 dimensions, so the morning can price each signal separately before deciding what stays in the prompt — the issue's stated condition for landing any of them. Both key on the OFFERING while the route holds the abstract `course_id`; `services/academics.py` bridges it once for both, and an empty resolution is reported through `tool_signals.report_empty_result` — a student with graph nodes in a course but no offering of it has every offering-keyed input dark, which is the #553 keyspace shape, not "nothing yet". No migrations. No encrypted column is selected (`flashcards.front`/`back`, `messages.content`). Every read is bounded, owner-scoped, and degrades to unknown rather than to zero. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(quiz): share the course lookups the H4 signals duplicated (#556 review) Review round 1 on the two new H4 signals. Six new round-trips per generation became three, plus four smaller fixes. Three of the six re-asked questions a CONCURRENT leg of the same request was already asking, so nothing could be shared by accident: * `user_offering_ids_for_course` (two UNCACHED reads) — `exam_proximity` wants it too; * the `courses` row — grounding reads `course_code` off it, the flashcard signal reads `course_name` off the same row. `_quiz_via_agent` now resolves both up front (concurrently with each other) and injects them: `CourseRow` into `_course_material`/`_resolve_bu_code`, `offering_ids` into `days_until_next_exam`, and a `CourseScope` into `gather_signals`. Every injection point is an OPTIONAL argument, so existing callers resolve exactly as before. M2: the F5 probe was a tautology — `HAS_GRAPH` scoped to the course asks what the route answered by reading the node on the way in. `report_empty_result` gains an optional `plausible` for callers that hold the fact, which skips the probe read while keeping the `quiz.tool_empty` signal and its expectation. M3: the flashcard read now narrows to one course in PostgREST via an `or=` tree (`offering_id.in.(…)`, `topic.ilike."<course>"`) instead of scanning the student's whole collection and filtering in Python. The count comes from Content-Range, so it is exact no matter what the cap does, and the newest- review ordering keeps the recency answer correct on a truncated page; only the reviewed tally goes unknown there. M5: `_days_since` parses through `routes.quiz._parse_ts` rather than keeping a second copy of the naive/aware rule. M6: a test pins that the ordinary empty path — scope resolves, both reads empty — reports NOTHING. Zero flashcards is what a first-week student looks like, and firing there is the alarm fatigue `tool_signals` exists to avoid. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(quiz): close the three gaps the H4 read-sharing opened (#556 review 2) A — the hoisted resolution had lost the gather's exception backstop. It ran under a bare `asyncio.gather`, so a raise from the to_thread machinery (the one failure `_course_row`/`offering_scope` cannot swallow themselves) escaped `_quiz_via_agent` and 502'd QUIZ_GENERATION_FAILED instead of degrading. `return_exceptions=True` plus the two guards the sibling gather already uses: a failed course lookup becomes `CourseRow(failed=True)` (so E8 still reports coverage_unknown rather than "no BU code"), a failed resolution becomes None (not [], which would assert no enrollment and trip the F5 dark-scope report on a transport failure). B — a failed offering resolution reported a partial flashcard count as a fact. Round 1 made `_course_scope` always return a CourseScope, and the call site no longer gated on it: `offering_ids=None` plus a known course name built a topic-only `or=` tree and counted the generated decks while silently omitting every imported one. Both signals are now gated on `offering_ids is not None` — the situation the module's contract calls unknown. C — `%` and `_` in a course name are LIKE wildcards, and `\` is LIKE's escape character, so `topic.ilike."Math_101"` also matched "Math-101". `_like_literal` escapes all three BEFORE `_pg_quote` doubles the backslashes for PostgREST; the other order emits a bare `\%` that PostgREST unescapes back into a live wildcard. Each fix has a test verified to fail without it (502; a partial count of 1 reported as a fact; the unescaped pattern). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(quiz): read the H4 signals in the keyspace their tables are written in (#556) Merge-gate review of #592. Both new signals resolved their offering scope with `user_offering_ids_for_course` — ENROLLMENT-derived — while every writer of the tables they read stamps an enrollment-agnostic `resolve_offering(course_id, ...)`: * sessions -> resolve_offering(course_id, create=True) (learn.py:431,:882) * flashcards (import) -> resolve_offering(course_id) (flashcards.py:419) * flashcards (generate)-> no offering_id at all, only a free-text topic Neither table has a `course_id` column to fall back on (0025 recreated both without one). After a term rollover the two keyspaces diverge permanently — the no-retake rule keeps the enrollment in the old term while new sessions and imports land on the current term's offering — so an engaged student read 0 sessions AS A FACT, with no F5 report because the offering list was not empty, just wrong. That is the #553/#529 shape this module's own docstring cites. `course_offering_ids(course_id)` now returns EVERY offering of the abstract course: the exact closure of what those writers can stamp, one read instead of two, and no weaker on ownership (both tables carry `user_id` and both reads filter on it). exam_proximity takes the wider set safely — it narrows back to enrollments itself, because `assignments` is enrollment-keyed. Also, the degraded paths that presented an undercount as a verified fact: * flashcards are SKIPPED when the `courses` read RAISED (new `CourseScope.name_failed`), instead of running an offering-only tree that cannot see an AI-generated card; the route's WARNING now says which half it lost rather than claiming all of them * the topic clause is a SUBSTRING match, matching what Study.tsx counts under a course; equality missed every imported deck whose topic is not verbatim the course name (it is a text box the student types into) * the session window is "started in the last 14d OR never ended", so the resume flow's long-lived sessions stop reading as no tutoring — and the prompt line says "or still open" rather than asserting the window * `course_offering_ids` logs a real DB failure at WARNING, not debug, and reports a truncated offering read as unknown rather than a partial list * `plausible=True` is no longer hard-coded into the F5 dark-scope report: the route asserts the graph row it read, `scripts/benchmark_quiz.py` (fixture user, no graph) asserts nothing and the probe decides, and the probe is scoped to the course * `_days_since` counts CALENDAR days, agreeing with the exam line one sentence away instead of disagreeing for most of every day Cleanups the review asked for: `pg_quote_value`/`like_literal` move to db/connection.py as PostgREST grammar and fix routes/onboarding.py's course search, which shipped the bug they exist to prevent; `parse_ts` moves to services/timestamps.py so a service no longer imports a route; the dead, divergent `_course_scope` is gone and `scope` is a required argument; `exam_prompt_line` is extracted so the prompt-budget benchmark measures the real sentence instead of a hand-copy that was two features stale; the budget doc's NULL/zero pricing advice is corrected (it was inverted on both halves). Refs #556, #537. Follow-up filed as #596 (node_mastery_events as the primary tutor-recency read). * refactor(academics): move course_offering_ids to its CLAUDE.md home (#556) Re-review of the #592 merge-gate rework: `course_offering_ids` is term/offering resolution reading `table("course_offerings")`, and CLAUDE.md names `services/academics.py` as the single home for that — a feature module was the wrong place for it, and `exam_proximity` had ended up pointing at `quiz_signals` for an offering concept. Moved verbatim (WARNING + exc_info logging and the scan cap came with it), next to `user_offering_ids_for_course` so the two keyspaces are readable side by side, with each docstring naming when the other is the right one. `routes/quiz.py` imports it from academics now; `quiz_signals` no longer resolves offerings at all — it consumes the injected `CourseScope`. Its unit tests moved to `tests/test_academics.py` with them. `user_offering_ids_for_course` does NOT reuse it, and that is deliberate rather than an omission. Sharing the read means sharing `select_with_count`, and the two want different failure semantics: the counted form never raises and answers `None` for "couldn't tell", while the enrollment form has no tri-state and must let a failed read raise, because degrading to `[]` there would assert the student is enrolled in nothing. Routing one through the other either swallows that or forces the counted form to raise — and in practice the swap also broke 26 tests across gradebook/graph/study-guide that mock only `.select`, plus it would have turned a silently-truncated read into an exception inside `enrollment_id_for`. One duplicated `select` line is the cheaper trade; both docstrings say so, and a new test pins the raising behaviour so a future merge cannot quietly erase it. Two wording fixes the re-review also caught: * `_course_row`'s own WARNING is the one that fires in the common failure path, so it now names both consumers of that row — grounding coverage AND the flashcard signals — instead of grounding alone; * `TestSignalCourseKeyspace::test_the_scope_never_consults_enrollments` overstated what it checked (it asserts the `course_offerings` read happened; it cannot assert `enrollments` was untouched, because the exam leg reads it legitimately). Renamed to `test_the_scope_is_resolved_from_the_courses_own_offerings` and pointed at the module-level twin in `tests/test_academics.py`, which does assert absence properly. Refs #556, #537. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

AndresL230 · 2026-08-22T10:14:46Z

Closes#556 (four of five — see Scope below). Workstream H4 of epic #537.

Each of these was already sitting in a table the generation path touches, and none of it reached the prompt.

signal	cost
`graph_nodes.times_studied`	zero queries — already in the row `generate_quiz` reads to resolve the concept name and course, just never selected
learning velocity	one bounded read; `graph_service._compute_velocity`imported, not re-derived
unfinished attempts on this concept	one owner-scoped read, reusing #542's derived-status rule
flashcards for this concept	one read, a count — front/back are encrypted (#518) and neither read nor needed

times_studied is passed into the gatherer rather than re-read: a second query would spend a round-trip to learn something we were already told.

Landed behind the measurement, as the issue requires

Every signal records its own dimension on quiz.started, which shares a request_id with the llm_usage row — so "is this worth its tokens?" is answerable with data rather than taste. That was the issue's stated condition for landing them at all.

It runs as a fourth leg of the gather that already resolves grounding, the recently-asked list and exam proximity — best-effort context exactly like those three, so it adds no latency. And it never raises: a signal nobody strictly needs must not be able to fail a generation.

Two distinctions the code keeps deliberately

None (couldn't tell) is not 0 (a fact about the student). Collapsing them would report "never studied" for a broken query.
A zero velocity is not reported, because _compute_velocity returns 0.0 both for "no recent gain" and "not enough data" — claiming stagnation we can't distinguish from silence. A zero times_studiedis reported: that one is a fact, and exactly what a generator should know.

Scope — one item deliberately left out

Tutor recency via messages.graph_update_json is not here. The issue frames all five as "one query or one extra selected column", and that one isn't: messages has no user_id (it's session-scoped), so it needs a sessions → messages join plus JSONB concept-name matching through PostgREST. That's a different size of change with a different cost profile, and bolting it on would have made the F6 numbers for the other four harder to read.

Left out deliberately rather than done badly. Flagged on the issue so it doesn't evaporate.

Verification

Hermetic 2218 passed / 9 skipped, ruff clean, oracles 0 findings, integration 70 passed, Playwright 47 passed (the one failure is #566, fixed by #568).

🤖 Generated with Claude Code

Four of the five blind spots the issue lists. Each was already sitting in a table the generation path touches, and none of it reached the prompt: * `graph_nodes.times_studied` — in the row `generate_quiz` ALREADY reads to resolve the concept name and course, simply never selected. One more column, zero extra queries; passed INTO the gatherer rather than re-read, because a second read would spend a round-trip to learn something we were already told. * learning velocity — `graph_service._compute_velocity` is imported, not re-derived. #557 is what two copies of one number costs. * unfinished attempts on this concept — trivially available since #542 gave attempts a derived status; the same `completed_at IS NULL` rule, not a second definition of "unfinished". * flashcards for this concept — a COUNT. Front/back are encrypted (#518) and are neither read nor needed. Landed BEHIND the F6 measurement, which is the condition the issue sets: every signal records its own prompt dimension on `quiz.started`, which shares a `request_id` with the `llm_usage` row, so "is this worth its tokens?" is answerable with data rather than taste. Runs as a fourth leg of the gather that already resolves grounding, the recently-asked list and exam proximity — best-effort context exactly like those three, so it costs no added latency. Never raises: a signal nobody strictly needs must not be able to fail a generation. Two distinctions the code keeps deliberately: * `None` (could not tell) is not `0` (a fact about the student). Collapsing them would report "never studied" for a broken query. * a zero VELOCITY is not reported, because `_compute_velocity` returns 0.0 both for "no recent gain" and "not enough data" — claiming stagnation we cannot distinguish from silence. A zero `times_studied` IS reported: that one is a fact, and exactly what a generator should know. NOT included: tutor recency via `messages.graph_update_json`. The issue frames all five as "one query or one extra selected column", and that one isn't: `messages` has no `user_id` (it is session-scoped), so it needs a sessions -> messages join plus JSONB concept-name matching through PostgREST. Left out deliberately rather than done badly; noted on the issue. Hermetic 2218 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

cloudflare-workers-and-pages · 2026-08-22T10:14:51Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	frontend-staging	`ee1c91e`	Commit Preview URL Branch Preview URL	Aug 22 2026, 10:23 AM

coderabbitai · 2026-08-22T10:14:58Z

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:19 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 @coderabbitai review or push new commits to the PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f78995c1-90ee-4268-a513-4ab94cab82bd

📥 Commits

Reviewing files that changed from the base of the PR and between b14fc3b and ee1c91e.

📒 Files selected for processing (4)

backend/routes/quiz.py
backend/services/quiz_signals.py
backend/tests/test_event_capture_seams.py
backend/tests/test_quiz_signals.py

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

supabase · 2026-08-22T10:17:40Z

This pull request has been ignored for the connected project ybgqdonkoqftwrmweuyv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

Five findings. The first is the one this module was written to avoid. **The velocity was inflated up to 14x, and disagreed with the Tree.** `_compute_velocity` derives its window from `recent[0]` as the OLDEST recent event — which is why `get_graph` feeds it `created_at.asc`. I read `desc` (correct, so the LIMIT keeps the NEWEST events) and handed the rows straight over, collapsing `days` to `max(1, 0) == 1`. Four events of +0.1 over ten days: 0.04 on the graph screen, 0.4 in the quiz prompt. So I reintroduced the exact "two copies of one number" failure the docstring cites #557 for — not by copying the algorithm, but by breaking its contract through the argument. Rows are reversed now, and a test pins the two against each other rather than against a literal. **The flashcard signal is removed, not fixed.** It matched `flashcards.topic` against a concept name, and nothing ever writes a concept name there: every writer stores `body.topic`, and the only caller (`Study.tsx`) passes `course.course_name`. So it was a permanent zero plus a wasted request-path round-trip — and worse, it would have had the F6 measurement conclude the signal is worthless when it was simply never wired to real data, which defeats the entire point of landing these behind a measurement. It needs a concept<->card mapping first; noted in the module and on the issue. **In-flight now shares #542's TTL.** Checking only `completed_at`/ `abandoned_at` IS NULL is a second, looser definition of "unfinished": `_attempt_status` treats an in-progress row past the 24h TTL as abandoned even before the lazy sweep stamps it, and that sweep runs on the history reads, not on generation. A student who generated three quizzes last week and never opened their history would have been told they had three in flight. **Counts are exact, not saturated.** `len(rows)` under a LIMIT reported the cap as though it were a fact; `select_with_count` gives the true total for the same round-trip. **The H3 comment was orphaned** — the H4 block had been inserted between the exam-proximity rationale and the code it explains. Moved below it. Also value-checked the new dimensions in the `quiz.started` pin, which had been comparing the payload against itself and so asserted presence but never correctness. Hermetic 2219 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

AndresL230 · 2026-08-22T10:26:32Z

Review round — five findings, and the first one is the irony of this PR

I broke _compute_velocity by reusing it. The module docstring cites #557 for why the 14-day computation is imported rather than copied — and then I fed it rows in the opposite order from its only other caller. _compute_velocity derives its window from recent[0] as the oldest recent event, which is why get_graph reads created_at.asc. I read desc (correct, so the LIMIT keeps the newest events) and handed them straight over, collapsing days to max(1, 0) == 1.

Four events of +0.1 over ten days:

surface	velocity
Tree / graph screen	0.04
quiz prompt (mine)	0.4

Same concept, two numbers — the exact failure #557 spent a workstream undoing, reintroduced through the argument instead of the algorithm. Rows are reversed now, and the test pins the two computations against each other rather than against a literal.

The flashcard signal is removed, not fixed. It matched flashcards.topic against a concept name, and nothing ever writes a concept name there: routes/flashcards.py stores body.topic, and the only caller (Study.tsx:711) passes course.course_name. So it was a permanent zero plus a wasted request-path round-trip — and worse, it would have had the F6 measurement conclude the signal is worthless when it was simply never wired to real data. That defeats the entire point of landing these behind a measurement. It needs a concept↔card mapping first; documented in the module and flagged on the issue.

In-flight now shares #542's TTL. Checking only completed_at/abandoned_at IS NULL is a second, looser definition of "unfinished": _attempt_status calls an in-progress row past the 24h TTL abandoned even before the lazy sweep stamps it, and that sweep runs on the history reads — not on generation. A student who generated three quizzes last week and never opened their history would have been told they had three in flight.

Counts are exact, not saturated.len(rows) under a LIMIT reported the cap as though it were a fact about the student; select_with_count gives the true total for the same round-trip.

The H3 comment was orphaned — my H4 block had been inserted between the exam-proximity rationale and the code it explains. Moved below it.

Also value-checked the new dimensions in the quiz.started pin, which the review noted were comparing the payload against itself — asserting presence but never correctness.

Scope, restated

This closes three of the five signals #556 lists (times_studied, velocity, in-flight attempts). Two are deliberately not here, both for the same reason — they'd measure nothing as specified:

flashcard review state — no concept-level data exists to read (above);
tutor recency — messages has no user_id, so it needs a sessions → messages join plus JSONB concept matching, which is not "one query" like the others.

Verification

Hermetic 2219 passed / 9 skipped, ruff clean, oracles 0 findings, integration 70 passed, Playwright 47 passed (the one failure is #566, fixed by #568). CI green.

…pports (#556) (#592) * feat(quiz): land the two deferred H4 signals at the scope the data supports (#556) #575 shipped three of the five H4 personalization signals and deferred the other two because, as the issue specified them, both would have measured nothing: * flashcards have no concept link — `topic` is free text every writer sets to the COURSE name — so a concept-name match is a permanent zero; * tutor recency needs a sessions -> messages join (`messages` has no `user_id`) plus JSONB concept matching. Both now land: the first at COURSE scope rather than pretending to concept scope, the second as one bounded owner-scoped `sessions` read plus one `messages` read over the ids it returns. Five new fields, five new F6 dimensions, so the morning can price each signal separately before deciding what stays in the prompt — the issue's stated condition for landing any of them. Both key on the OFFERING while the route holds the abstract `course_id`; `services/academics.py` bridges it once for both, and an empty resolution is reported through `tool_signals.report_empty_result` — a student with graph nodes in a course but no offering of it has every offering-keyed input dark, which is the #553 keyspace shape, not "nothing yet". No migrations. No encrypted column is selected (`flashcards.front`/`back`, `messages.content`). Every read is bounded, owner-scoped, and degrades to unknown rather than to zero. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(quiz): share the course lookups the H4 signals duplicated (#556 review) Review round 1 on the two new H4 signals. Six new round-trips per generation became three, plus four smaller fixes. Three of the six re-asked questions a CONCURRENT leg of the same request was already asking, so nothing could be shared by accident: * `user_offering_ids_for_course` (two UNCACHED reads) — `exam_proximity` wants it too; * the `courses` row — grounding reads `course_code` off it, the flashcard signal reads `course_name` off the same row. `_quiz_via_agent` now resolves both up front (concurrently with each other) and injects them: `CourseRow` into `_course_material`/`_resolve_bu_code`, `offering_ids` into `days_until_next_exam`, and a `CourseScope` into `gather_signals`. Every injection point is an OPTIONAL argument, so existing callers resolve exactly as before. M2: the F5 probe was a tautology — `HAS_GRAPH` scoped to the course asks what the route answered by reading the node on the way in. `report_empty_result` gains an optional `plausible` for callers that hold the fact, which skips the probe read while keeping the `quiz.tool_empty` signal and its expectation. M3: the flashcard read now narrows to one course in PostgREST via an `or=` tree (`offering_id.in.(…)`, `topic.ilike."<course>"`) instead of scanning the student's whole collection and filtering in Python. The count comes from Content-Range, so it is exact no matter what the cap does, and the newest- review ordering keeps the recency answer correct on a truncated page; only the reviewed tally goes unknown there. M5: `_days_since` parses through `routes.quiz._parse_ts` rather than keeping a second copy of the naive/aware rule. M6: a test pins that the ordinary empty path — scope resolves, both reads empty — reports NOTHING. Zero flashcards is what a first-week student looks like, and firing there is the alarm fatigue `tool_signals` exists to avoid. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(quiz): close the three gaps the H4 read-sharing opened (#556 review 2) A — the hoisted resolution had lost the gather's exception backstop. It ran under a bare `asyncio.gather`, so a raise from the to_thread machinery (the one failure `_course_row`/`offering_scope` cannot swallow themselves) escaped `_quiz_via_agent` and 502'd QUIZ_GENERATION_FAILED instead of degrading. `return_exceptions=True` plus the two guards the sibling gather already uses: a failed course lookup becomes `CourseRow(failed=True)` (so E8 still reports coverage_unknown rather than "no BU code"), a failed resolution becomes None (not [], which would assert no enrollment and trip the F5 dark-scope report on a transport failure). B — a failed offering resolution reported a partial flashcard count as a fact. Round 1 made `_course_scope` always return a CourseScope, and the call site no longer gated on it: `offering_ids=None` plus a known course name built a topic-only `or=` tree and counted the generated decks while silently omitting every imported one. Both signals are now gated on `offering_ids is not None` — the situation the module's contract calls unknown. C — `%` and `_` in a course name are LIKE wildcards, and `\` is LIKE's escape character, so `topic.ilike."Math_101"` also matched "Math-101". `_like_literal` escapes all three BEFORE `_pg_quote` doubles the backslashes for PostgREST; the other order emits a bare `\%` that PostgREST unescapes back into a live wildcard. Each fix has a test verified to fail without it (502; a partial count of 1 reported as a fact; the unescaped pattern). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(quiz): read the H4 signals in the keyspace their tables are written in (#556) Merge-gate review of #592. Both new signals resolved their offering scope with `user_offering_ids_for_course` — ENROLLMENT-derived — while every writer of the tables they read stamps an enrollment-agnostic `resolve_offering(course_id, ...)`: * sessions -> resolve_offering(course_id, create=True) (learn.py:431,:882) * flashcards (import) -> resolve_offering(course_id) (flashcards.py:419) * flashcards (generate)-> no offering_id at all, only a free-text topic Neither table has a `course_id` column to fall back on (0025 recreated both without one). After a term rollover the two keyspaces diverge permanently — the no-retake rule keeps the enrollment in the old term while new sessions and imports land on the current term's offering — so an engaged student read 0 sessions AS A FACT, with no F5 report because the offering list was not empty, just wrong. That is the #553/#529 shape this module's own docstring cites. `course_offering_ids(course_id)` now returns EVERY offering of the abstract course: the exact closure of what those writers can stamp, one read instead of two, and no weaker on ownership (both tables carry `user_id` and both reads filter on it). exam_proximity takes the wider set safely — it narrows back to enrollments itself, because `assignments` is enrollment-keyed. Also, the degraded paths that presented an undercount as a verified fact: * flashcards are SKIPPED when the `courses` read RAISED (new `CourseScope.name_failed`), instead of running an offering-only tree that cannot see an AI-generated card; the route's WARNING now says which half it lost rather than claiming all of them * the topic clause is a SUBSTRING match, matching what Study.tsx counts under a course; equality missed every imported deck whose topic is not verbatim the course name (it is a text box the student types into) * the session window is "started in the last 14d OR never ended", so the resume flow's long-lived sessions stop reading as no tutoring — and the prompt line says "or still open" rather than asserting the window * `course_offering_ids` logs a real DB failure at WARNING, not debug, and reports a truncated offering read as unknown rather than a partial list * `plausible=True` is no longer hard-coded into the F5 dark-scope report: the route asserts the graph row it read, `scripts/benchmark_quiz.py` (fixture user, no graph) asserts nothing and the probe decides, and the probe is scoped to the course * `_days_since` counts CALENDAR days, agreeing with the exam line one sentence away instead of disagreeing for most of every day Cleanups the review asked for: `pg_quote_value`/`like_literal` move to db/connection.py as PostgREST grammar and fix routes/onboarding.py's course search, which shipped the bug they exist to prevent; `parse_ts` moves to services/timestamps.py so a service no longer imports a route; the dead, divergent `_course_scope` is gone and `scope` is a required argument; `exam_prompt_line` is extracted so the prompt-budget benchmark measures the real sentence instead of a hand-copy that was two features stale; the budget doc's NULL/zero pricing advice is corrected (it was inverted on both halves). Refs #556, #537. Follow-up filed as #596 (node_mastery_events as the primary tutor-recency read). * refactor(academics): move course_offering_ids to its CLAUDE.md home (#556) Re-review of the #592 merge-gate rework: `course_offering_ids` is term/offering resolution reading `table("course_offerings")`, and CLAUDE.md names `services/academics.py` as the single home for that — a feature module was the wrong place for it, and `exam_proximity` had ended up pointing at `quiz_signals` for an offering concept. Moved verbatim (WARNING + exc_info logging and the scan cap came with it), next to `user_offering_ids_for_course` so the two keyspaces are readable side by side, with each docstring naming when the other is the right one. `routes/quiz.py` imports it from academics now; `quiz_signals` no longer resolves offerings at all — it consumes the injected `CourseScope`. Its unit tests moved to `tests/test_academics.py` with them. `user_offering_ids_for_course` does NOT reuse it, and that is deliberate rather than an omission. Sharing the read means sharing `select_with_count`, and the two want different failure semantics: the counted form never raises and answers `None` for "couldn't tell", while the enrollment form has no tri-state and must let a failed read raise, because degrading to `[]` there would assert the student is enrolled in nothing. Routing one through the other either swallows that or forces the counted form to raise — and in practice the swap also broke 26 tests across gradebook/graph/study-guide that mock only `.select`, plus it would have turned a silently-truncated read into an exception inside `enrollment_id_for`. One duplicated `select` line is the cheaper trade; both docstrings say so, and a new test pins the raising behaviour so a future merge cannot quietly erase it. Two wording fixes the re-review also caught: * `_course_row`'s own WARNING is the one that fires in the common failure path, so it now names both consumers of that row — grounding coverage AND the flashcard signals — instead of grounding alone; * `TestSignalCourseKeyspace::test_the_scope_never_consults_enrollments` overstated what it checked (it asserts the `course_offerings` read happened; it cannot assert `enrollments` was untouched, because the exam leg reads it legitimately). Renamed to `test_the_scope_is_resolved_from_the_courses_own_offerings` and pointed at the module-level twin in `tests/test_academics.py`, which does assert absence properly. Refs #556, #537. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

AndresL230 · 2026-08-22T10:14:46Z

Closes#556 (four of five — see Scope below). Workstream H4 of epic #537.

Each of these was already sitting in a table the generation path touches, and none of it reached the prompt.

signal	cost
`graph_nodes.times_studied`	zero queries — already in the row `generate_quiz` reads to resolve the concept name and course, just never selected
learning velocity	one bounded read; `graph_service._compute_velocity`imported, not re-derived
unfinished attempts on this concept	one owner-scoped read, reusing #542's derived-status rule
flashcards for this concept	one read, a count — front/back are encrypted (#518) and neither read nor needed

times_studied is passed into the gatherer rather than re-read: a second query would spend a round-trip to learn something we were already told.

Landed behind the measurement, as the issue requires

Every signal records its own dimension on quiz.started, which shares a request_id with the llm_usage row — so "is this worth its tokens?" is answerable with data rather than taste. That was the issue's stated condition for landing them at all.

It runs as a fourth leg of the gather that already resolves grounding, the recently-asked list and exam proximity — best-effort context exactly like those three, so it adds no latency. And it never raises: a signal nobody strictly needs must not be able to fail a generation.

Two distinctions the code keeps deliberately

None (couldn't tell) is not 0 (a fact about the student). Collapsing them would report "never studied" for a broken query.
A zero velocity is not reported, because _compute_velocity returns 0.0 both for "no recent gain" and "not enough data" — claiming stagnation we can't distinguish from silence. A zero times_studiedis reported: that one is a fact, and exactly what a generator should know.

Scope — one item deliberately left out

Tutor recency via messages.graph_update_json is not here. The issue frames all five as "one query or one extra selected column", and that one isn't: messages has no user_id (it's session-scoped), so it needs a sessions → messages join plus JSONB concept-name matching through PostgREST. That's a different size of change with a different cost profile, and bolting it on would have made the F6 numbers for the other four harder to read.

Left out deliberately rather than done badly. Flagged on the issue so it doesn't evaporate.

Verification

Hermetic 2218 passed / 9 skipped, ruff clean, oracles 0 findings, integration 70 passed, Playwright 47 passed (the one failure is #566, fixed by #568).

🤖 Generated with Claude Code

Four of the five blind spots the issue lists. Each was already sitting in a table the generation path touches, and none of it reached the prompt: * `graph_nodes.times_studied` — in the row `generate_quiz` ALREADY reads to resolve the concept name and course, simply never selected. One more column, zero extra queries; passed INTO the gatherer rather than re-read, because a second read would spend a round-trip to learn something we were already told. * learning velocity — `graph_service._compute_velocity` is imported, not re-derived. #557 is what two copies of one number costs. * unfinished attempts on this concept — trivially available since #542 gave attempts a derived status; the same `completed_at IS NULL` rule, not a second definition of "unfinished". * flashcards for this concept — a COUNT. Front/back are encrypted (#518) and are neither read nor needed. Landed BEHIND the F6 measurement, which is the condition the issue sets: every signal records its own prompt dimension on `quiz.started`, which shares a `request_id` with the `llm_usage` row, so "is this worth its tokens?" is answerable with data rather than taste. Runs as a fourth leg of the gather that already resolves grounding, the recently-asked list and exam proximity — best-effort context exactly like those three, so it costs no added latency. Never raises: a signal nobody strictly needs must not be able to fail a generation. Two distinctions the code keeps deliberately: * `None` (could not tell) is not `0` (a fact about the student). Collapsing them would report "never studied" for a broken query. * a zero VELOCITY is not reported, because `_compute_velocity` returns 0.0 both for "no recent gain" and "not enough data" — claiming stagnation we cannot distinguish from silence. A zero `times_studied` IS reported: that one is a fact, and exactly what a generator should know. NOT included: tutor recency via `messages.graph_update_json`. The issue frames all five as "one query or one extra selected column", and that one isn't: `messages` has no `user_id` (it is session-scoped), so it needs a sessions -> messages join plus JSONB concept-name matching through PostgREST. Left out deliberately rather than done badly; noted on the issue. Hermetic 2218 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

cloudflare-workers-and-pages · 2026-08-22T10:14:51Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	frontend-staging	`ee1c91e`	Commit Preview URL Branch Preview URL	Aug 22 2026, 10:23 AM

coderabbitai · 2026-08-22T10:14:58Z

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:19 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 @coderabbitai review or push new commits to the PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f78995c1-90ee-4268-a513-4ab94cab82bd

📥 Commits

Reviewing files that changed from the base of the PR and between b14fc3b and ee1c91e.

📒 Files selected for processing (4)

backend/routes/quiz.py
backend/services/quiz_signals.py
backend/tests/test_event_capture_seams.py
backend/tests/test_quiz_signals.py

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

supabase · 2026-08-22T10:17:40Z

This pull request has been ignored for the connected project ybgqdonkoqftwrmweuyv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

Five findings. The first is the one this module was written to avoid. **The velocity was inflated up to 14x, and disagreed with the Tree.** `_compute_velocity` derives its window from `recent[0]` as the OLDEST recent event — which is why `get_graph` feeds it `created_at.asc`. I read `desc` (correct, so the LIMIT keeps the NEWEST events) and handed the rows straight over, collapsing `days` to `max(1, 0) == 1`. Four events of +0.1 over ten days: 0.04 on the graph screen, 0.4 in the quiz prompt. So I reintroduced the exact "two copies of one number" failure the docstring cites #557 for — not by copying the algorithm, but by breaking its contract through the argument. Rows are reversed now, and a test pins the two against each other rather than against a literal. **The flashcard signal is removed, not fixed.** It matched `flashcards.topic` against a concept name, and nothing ever writes a concept name there: every writer stores `body.topic`, and the only caller (`Study.tsx`) passes `course.course_name`. So it was a permanent zero plus a wasted request-path round-trip — and worse, it would have had the F6 measurement conclude the signal is worthless when it was simply never wired to real data, which defeats the entire point of landing these behind a measurement. It needs a concept<->card mapping first; noted in the module and on the issue. **In-flight now shares #542's TTL.** Checking only `completed_at`/ `abandoned_at` IS NULL is a second, looser definition of "unfinished": `_attempt_status` treats an in-progress row past the 24h TTL as abandoned even before the lazy sweep stamps it, and that sweep runs on the history reads, not on generation. A student who generated three quizzes last week and never opened their history would have been told they had three in flight. **Counts are exact, not saturated.** `len(rows)` under a LIMIT reported the cap as though it were a fact; `select_with_count` gives the true total for the same round-trip. **The H3 comment was orphaned** — the H4 block had been inserted between the exam-proximity rationale and the code it explains. Moved below it. Also value-checked the new dimensions in the `quiz.started` pin, which had been comparing the payload against itself and so asserted presence but never correctness. Hermetic 2219 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

AndresL230 · 2026-08-22T10:26:32Z

Review round — five findings, and the first one is the irony of this PR

I broke _compute_velocity by reusing it. The module docstring cites #557 for why the 14-day computation is imported rather than copied — and then I fed it rows in the opposite order from its only other caller. _compute_velocity derives its window from recent[0] as the oldest recent event, which is why get_graph reads created_at.asc. I read desc (correct, so the LIMIT keeps the newest events) and handed them straight over, collapsing days to max(1, 0) == 1.

Four events of +0.1 over ten days:

surface	velocity
Tree / graph screen	0.04
quiz prompt (mine)	0.4

Same concept, two numbers — the exact failure #557 spent a workstream undoing, reintroduced through the argument instead of the algorithm. Rows are reversed now, and the test pins the two computations against each other rather than against a literal.

The flashcard signal is removed, not fixed. It matched flashcards.topic against a concept name, and nothing ever writes a concept name there: routes/flashcards.py stores body.topic, and the only caller (Study.tsx:711) passes course.course_name. So it was a permanent zero plus a wasted request-path round-trip — and worse, it would have had the F6 measurement conclude the signal is worthless when it was simply never wired to real data. That defeats the entire point of landing these behind a measurement. It needs a concept↔card mapping first; documented in the module and flagged on the issue.

In-flight now shares #542's TTL. Checking only completed_at/abandoned_at IS NULL is a second, looser definition of "unfinished": _attempt_status calls an in-progress row past the 24h TTL abandoned even before the lazy sweep stamps it, and that sweep runs on the history reads — not on generation. A student who generated three quizzes last week and never opened their history would have been told they had three in flight.

Counts are exact, not saturated.len(rows) under a LIMIT reported the cap as though it were a fact about the student; select_with_count gives the true total for the same round-trip.

The H3 comment was orphaned — my H4 block had been inserted between the exam-proximity rationale and the code it explains. Moved below it.

Also value-checked the new dimensions in the quiz.started pin, which the review noted were comparing the payload against itself — asserting presence but never correctness.

Scope, restated

This closes three of the five signals #556 lists (times_studied, velocity, in-flight attempts). Two are deliberately not here, both for the same reason — they'd measure nothing as specified:

flashcard review state — no concept-level data exists to read (above);
tutor recency — messages has no user_id, so it needs a sessions → messages join plus JSONB concept matching, which is not "one query" like the others.

Verification

Hermetic 2219 passed / 9 skipped, ruff clean, oracles 0 findings, integration 70 passed, Playwright 47 passed (the one failure is #566, fixed by #568). CI green.

…pports (#556) (#592) * feat(quiz): land the two deferred H4 signals at the scope the data supports (#556) #575 shipped three of the five H4 personalization signals and deferred the other two because, as the issue specified them, both would have measured nothing: * flashcards have no concept link — `topic` is free text every writer sets to the COURSE name — so a concept-name match is a permanent zero; * tutor recency needs a sessions -> messages join (`messages` has no `user_id`) plus JSONB concept matching. Both now land: the first at COURSE scope rather than pretending to concept scope, the second as one bounded owner-scoped `sessions` read plus one `messages` read over the ids it returns. Five new fields, five new F6 dimensions, so the morning can price each signal separately before deciding what stays in the prompt — the issue's stated condition for landing any of them. Both key on the OFFERING while the route holds the abstract `course_id`; `services/academics.py` bridges it once for both, and an empty resolution is reported through `tool_signals.report_empty_result` — a student with graph nodes in a course but no offering of it has every offering-keyed input dark, which is the #553 keyspace shape, not "nothing yet". No migrations. No encrypted column is selected (`flashcards.front`/`back`, `messages.content`). Every read is bounded, owner-scoped, and degrades to unknown rather than to zero. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(quiz): share the course lookups the H4 signals duplicated (#556 review) Review round 1 on the two new H4 signals. Six new round-trips per generation became three, plus four smaller fixes. Three of the six re-asked questions a CONCURRENT leg of the same request was already asking, so nothing could be shared by accident: * `user_offering_ids_for_course` (two UNCACHED reads) — `exam_proximity` wants it too; * the `courses` row — grounding reads `course_code` off it, the flashcard signal reads `course_name` off the same row. `_quiz_via_agent` now resolves both up front (concurrently with each other) and injects them: `CourseRow` into `_course_material`/`_resolve_bu_code`, `offering_ids` into `days_until_next_exam`, and a `CourseScope` into `gather_signals`. Every injection point is an OPTIONAL argument, so existing callers resolve exactly as before. M2: the F5 probe was a tautology — `HAS_GRAPH` scoped to the course asks what the route answered by reading the node on the way in. `report_empty_result` gains an optional `plausible` for callers that hold the fact, which skips the probe read while keeping the `quiz.tool_empty` signal and its expectation. M3: the flashcard read now narrows to one course in PostgREST via an `or=` tree (`offering_id.in.(…)`, `topic.ilike."<course>"`) instead of scanning the student's whole collection and filtering in Python. The count comes from Content-Range, so it is exact no matter what the cap does, and the newest- review ordering keeps the recency answer correct on a truncated page; only the reviewed tally goes unknown there. M5: `_days_since` parses through `routes.quiz._parse_ts` rather than keeping a second copy of the naive/aware rule. M6: a test pins that the ordinary empty path — scope resolves, both reads empty — reports NOTHING. Zero flashcards is what a first-week student looks like, and firing there is the alarm fatigue `tool_signals` exists to avoid. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(quiz): close the three gaps the H4 read-sharing opened (#556 review 2) A — the hoisted resolution had lost the gather's exception backstop. It ran under a bare `asyncio.gather`, so a raise from the to_thread machinery (the one failure `_course_row`/`offering_scope` cannot swallow themselves) escaped `_quiz_via_agent` and 502'd QUIZ_GENERATION_FAILED instead of degrading. `return_exceptions=True` plus the two guards the sibling gather already uses: a failed course lookup becomes `CourseRow(failed=True)` (so E8 still reports coverage_unknown rather than "no BU code"), a failed resolution becomes None (not [], which would assert no enrollment and trip the F5 dark-scope report on a transport failure). B — a failed offering resolution reported a partial flashcard count as a fact. Round 1 made `_course_scope` always return a CourseScope, and the call site no longer gated on it: `offering_ids=None` plus a known course name built a topic-only `or=` tree and counted the generated decks while silently omitting every imported one. Both signals are now gated on `offering_ids is not None` — the situation the module's contract calls unknown. C — `%` and `_` in a course name are LIKE wildcards, and `\` is LIKE's escape character, so `topic.ilike."Math_101"` also matched "Math-101". `_like_literal` escapes all three BEFORE `_pg_quote` doubles the backslashes for PostgREST; the other order emits a bare `\%` that PostgREST unescapes back into a live wildcard. Each fix has a test verified to fail without it (502; a partial count of 1 reported as a fact; the unescaped pattern). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(quiz): read the H4 signals in the keyspace their tables are written in (#556) Merge-gate review of #592. Both new signals resolved their offering scope with `user_offering_ids_for_course` — ENROLLMENT-derived — while every writer of the tables they read stamps an enrollment-agnostic `resolve_offering(course_id, ...)`: * sessions -> resolve_offering(course_id, create=True) (learn.py:431,:882) * flashcards (import) -> resolve_offering(course_id) (flashcards.py:419) * flashcards (generate)-> no offering_id at all, only a free-text topic Neither table has a `course_id` column to fall back on (0025 recreated both without one). After a term rollover the two keyspaces diverge permanently — the no-retake rule keeps the enrollment in the old term while new sessions and imports land on the current term's offering — so an engaged student read 0 sessions AS A FACT, with no F5 report because the offering list was not empty, just wrong. That is the #553/#529 shape this module's own docstring cites. `course_offering_ids(course_id)` now returns EVERY offering of the abstract course: the exact closure of what those writers can stamp, one read instead of two, and no weaker on ownership (both tables carry `user_id` and both reads filter on it). exam_proximity takes the wider set safely — it narrows back to enrollments itself, because `assignments` is enrollment-keyed. Also, the degraded paths that presented an undercount as a verified fact: * flashcards are SKIPPED when the `courses` read RAISED (new `CourseScope.name_failed`), instead of running an offering-only tree that cannot see an AI-generated card; the route's WARNING now says which half it lost rather than claiming all of them * the topic clause is a SUBSTRING match, matching what Study.tsx counts under a course; equality missed every imported deck whose topic is not verbatim the course name (it is a text box the student types into) * the session window is "started in the last 14d OR never ended", so the resume flow's long-lived sessions stop reading as no tutoring — and the prompt line says "or still open" rather than asserting the window * `course_offering_ids` logs a real DB failure at WARNING, not debug, and reports a truncated offering read as unknown rather than a partial list * `plausible=True` is no longer hard-coded into the F5 dark-scope report: the route asserts the graph row it read, `scripts/benchmark_quiz.py` (fixture user, no graph) asserts nothing and the probe decides, and the probe is scoped to the course * `_days_since` counts CALENDAR days, agreeing with the exam line one sentence away instead of disagreeing for most of every day Cleanups the review asked for: `pg_quote_value`/`like_literal` move to db/connection.py as PostgREST grammar and fix routes/onboarding.py's course search, which shipped the bug they exist to prevent; `parse_ts` moves to services/timestamps.py so a service no longer imports a route; the dead, divergent `_course_scope` is gone and `scope` is a required argument; `exam_prompt_line` is extracted so the prompt-budget benchmark measures the real sentence instead of a hand-copy that was two features stale; the budget doc's NULL/zero pricing advice is corrected (it was inverted on both halves). Refs #556, #537. Follow-up filed as #596 (node_mastery_events as the primary tutor-recency read). * refactor(academics): move course_offering_ids to its CLAUDE.md home (#556) Re-review of the #592 merge-gate rework: `course_offering_ids` is term/offering resolution reading `table("course_offerings")`, and CLAUDE.md names `services/academics.py` as the single home for that — a feature module was the wrong place for it, and `exam_proximity` had ended up pointing at `quiz_signals` for an offering concept. Moved verbatim (WARNING + exc_info logging and the scan cap came with it), next to `user_offering_ids_for_course` so the two keyspaces are readable side by side, with each docstring naming when the other is the right one. `routes/quiz.py` imports it from academics now; `quiz_signals` no longer resolves offerings at all — it consumes the injected `CourseScope`. Its unit tests moved to `tests/test_academics.py` with them. `user_offering_ids_for_course` does NOT reuse it, and that is deliberate rather than an omission. Sharing the read means sharing `select_with_count`, and the two want different failure semantics: the counted form never raises and answers `None` for "couldn't tell", while the enrollment form has no tri-state and must let a failed read raise, because degrading to `[]` there would assert the student is enrolled in nothing. Routing one through the other either swallows that or forces the counted form to raise — and in practice the swap also broke 26 tests across gradebook/graph/study-guide that mock only `.select`, plus it would have turned a silently-truncated read into an exception inside `enrollment_id_for`. One duplicated `select` line is the cheaper trade; both docstrings say so, and a new test pins the raising behaviour so a future merge cannot quietly erase it. Two wording fixes the re-review also caught: * `_course_row`'s own WARNING is the one that fires in the common failure path, so it now names both consumers of that row — grounding coverage AND the flashcard signals — instead of grounding alone; * `TestSignalCourseKeyspace::test_the_scope_never_consults_enrollments` overstated what it checked (it asserts the `course_offerings` read happened; it cannot assert `enrollments` was untouched, because the exam leg reads it legitimately). Renamed to `test_the_scope_is_resolved_from_the_courses_own_offerings` and pointed at the module-level twin in `tests/test_academics.py`, which does assert absence properly. Refs #556, #537. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

AndresL230 · 2026-08-22T10:14:46Z

Closes#556 (four of five — see Scope below). Workstream H4 of epic #537.

Each of these was already sitting in a table the generation path touches, and none of it reached the prompt.

signal	cost
`graph_nodes.times_studied`	zero queries — already in the row `generate_quiz` reads to resolve the concept name and course, just never selected
learning velocity	one bounded read; `graph_service._compute_velocity`imported, not re-derived
unfinished attempts on this concept	one owner-scoped read, reusing #542's derived-status rule
flashcards for this concept	one read, a count — front/back are encrypted (#518) and neither read nor needed

times_studied is passed into the gatherer rather than re-read: a second query would spend a round-trip to learn something we were already told.

Landed behind the measurement, as the issue requires

Every signal records its own dimension on quiz.started, which shares a request_id with the llm_usage row — so "is this worth its tokens?" is answerable with data rather than taste. That was the issue's stated condition for landing them at all.

It runs as a fourth leg of the gather that already resolves grounding, the recently-asked list and exam proximity — best-effort context exactly like those three, so it adds no latency. And it never raises: a signal nobody strictly needs must not be able to fail a generation.

Two distinctions the code keeps deliberately

None (couldn't tell) is not 0 (a fact about the student). Collapsing them would report "never studied" for a broken query.
A zero velocity is not reported, because _compute_velocity returns 0.0 both for "no recent gain" and "not enough data" — claiming stagnation we can't distinguish from silence. A zero times_studiedis reported: that one is a fact, and exactly what a generator should know.

Scope — one item deliberately left out

Tutor recency via messages.graph_update_json is not here. The issue frames all five as "one query or one extra selected column", and that one isn't: messages has no user_id (it's session-scoped), so it needs a sessions → messages join plus JSONB concept-name matching through PostgREST. That's a different size of change with a different cost profile, and bolting it on would have made the F6 numbers for the other four harder to read.

Left out deliberately rather than done badly. Flagged on the issue so it doesn't evaporate.

Verification

Hermetic 2218 passed / 9 skipped, ruff clean, oracles 0 findings, integration 70 passed, Playwright 47 passed (the one failure is #566, fixed by #568).

🤖 Generated with Claude Code

Four of the five blind spots the issue lists. Each was already sitting in a table the generation path touches, and none of it reached the prompt: * `graph_nodes.times_studied` — in the row `generate_quiz` ALREADY reads to resolve the concept name and course, simply never selected. One more column, zero extra queries; passed INTO the gatherer rather than re-read, because a second read would spend a round-trip to learn something we were already told. * learning velocity — `graph_service._compute_velocity` is imported, not re-derived. #557 is what two copies of one number costs. * unfinished attempts on this concept — trivially available since #542 gave attempts a derived status; the same `completed_at IS NULL` rule, not a second definition of "unfinished". * flashcards for this concept — a COUNT. Front/back are encrypted (#518) and are neither read nor needed. Landed BEHIND the F6 measurement, which is the condition the issue sets: every signal records its own prompt dimension on `quiz.started`, which shares a `request_id` with the `llm_usage` row, so "is this worth its tokens?" is answerable with data rather than taste. Runs as a fourth leg of the gather that already resolves grounding, the recently-asked list and exam proximity — best-effort context exactly like those three, so it costs no added latency. Never raises: a signal nobody strictly needs must not be able to fail a generation. Two distinctions the code keeps deliberately: * `None` (could not tell) is not `0` (a fact about the student). Collapsing them would report "never studied" for a broken query. * a zero VELOCITY is not reported, because `_compute_velocity` returns 0.0 both for "no recent gain" and "not enough data" — claiming stagnation we cannot distinguish from silence. A zero `times_studied` IS reported: that one is a fact, and exactly what a generator should know. NOT included: tutor recency via `messages.graph_update_json`. The issue frames all five as "one query or one extra selected column", and that one isn't: `messages` has no `user_id` (it is session-scoped), so it needs a sessions -> messages join plus JSONB concept-name matching through PostgREST. Left out deliberately rather than done badly; noted on the issue. Hermetic 2218 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

cloudflare-workers-and-pages · 2026-08-22T10:14:51Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	frontend-staging	`ee1c91e`	Commit Preview URL Branch Preview URL	Aug 22 2026, 10:23 AM

coderabbitai · 2026-08-22T10:14:58Z

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:19 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 @coderabbitai review or push new commits to the PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f78995c1-90ee-4268-a513-4ab94cab82bd

📥 Commits

Reviewing files that changed from the base of the PR and between b14fc3b and ee1c91e.

📒 Files selected for processing (4)

backend/routes/quiz.py
backend/services/quiz_signals.py
backend/tests/test_event_capture_seams.py
backend/tests/test_quiz_signals.py

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

supabase · 2026-08-22T10:17:40Z

This pull request has been ignored for the connected project ybgqdonkoqftwrmweuyv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

Five findings. The first is the one this module was written to avoid. **The velocity was inflated up to 14x, and disagreed with the Tree.** `_compute_velocity` derives its window from `recent[0]` as the OLDEST recent event — which is why `get_graph` feeds it `created_at.asc`. I read `desc` (correct, so the LIMIT keeps the NEWEST events) and handed the rows straight over, collapsing `days` to `max(1, 0) == 1`. Four events of +0.1 over ten days: 0.04 on the graph screen, 0.4 in the quiz prompt. So I reintroduced the exact "two copies of one number" failure the docstring cites #557 for — not by copying the algorithm, but by breaking its contract through the argument. Rows are reversed now, and a test pins the two against each other rather than against a literal. **The flashcard signal is removed, not fixed.** It matched `flashcards.topic` against a concept name, and nothing ever writes a concept name there: every writer stores `body.topic`, and the only caller (`Study.tsx`) passes `course.course_name`. So it was a permanent zero plus a wasted request-path round-trip — and worse, it would have had the F6 measurement conclude the signal is worthless when it was simply never wired to real data, which defeats the entire point of landing these behind a measurement. It needs a concept<->card mapping first; noted in the module and on the issue. **In-flight now shares #542's TTL.** Checking only `completed_at`/ `abandoned_at` IS NULL is a second, looser definition of "unfinished": `_attempt_status` treats an in-progress row past the 24h TTL as abandoned even before the lazy sweep stamps it, and that sweep runs on the history reads, not on generation. A student who generated three quizzes last week and never opened their history would have been told they had three in flight. **Counts are exact, not saturated.** `len(rows)` under a LIMIT reported the cap as though it were a fact; `select_with_count` gives the true total for the same round-trip. **The H3 comment was orphaned** — the H4 block had been inserted between the exam-proximity rationale and the code it explains. Moved below it. Also value-checked the new dimensions in the `quiz.started` pin, which had been comparing the payload against itself and so asserted presence but never correctness. Hermetic 2219 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

AndresL230 · 2026-08-22T10:26:32Z

Review round — five findings, and the first one is the irony of this PR

I broke _compute_velocity by reusing it. The module docstring cites #557 for why the 14-day computation is imported rather than copied — and then I fed it rows in the opposite order from its only other caller. _compute_velocity derives its window from recent[0] as the oldest recent event, which is why get_graph reads created_at.asc. I read desc (correct, so the LIMIT keeps the newest events) and handed them straight over, collapsing days to max(1, 0) == 1.

Four events of +0.1 over ten days:

surface	velocity
Tree / graph screen	0.04
quiz prompt (mine)	0.4

Same concept, two numbers — the exact failure #557 spent a workstream undoing, reintroduced through the argument instead of the algorithm. Rows are reversed now, and the test pins the two computations against each other rather than against a literal.

The flashcard signal is removed, not fixed. It matched flashcards.topic against a concept name, and nothing ever writes a concept name there: routes/flashcards.py stores body.topic, and the only caller (Study.tsx:711) passes course.course_name. So it was a permanent zero plus a wasted request-path round-trip — and worse, it would have had the F6 measurement conclude the signal is worthless when it was simply never wired to real data. That defeats the entire point of landing these behind a measurement. It needs a concept↔card mapping first; documented in the module and flagged on the issue.

In-flight now shares #542's TTL. Checking only completed_at/abandoned_at IS NULL is a second, looser definition of "unfinished": _attempt_status calls an in-progress row past the 24h TTL abandoned even before the lazy sweep stamps it, and that sweep runs on the history reads — not on generation. A student who generated three quizzes last week and never opened their history would have been told they had three in flight.

Counts are exact, not saturated.len(rows) under a LIMIT reported the cap as though it were a fact about the student; select_with_count gives the true total for the same round-trip.

The H3 comment was orphaned — my H4 block had been inserted between the exam-proximity rationale and the code it explains. Moved below it.

Also value-checked the new dimensions in the quiz.started pin, which the review noted were comparing the payload against itself — asserting presence but never correctness.

Scope, restated

This closes three of the five signals #556 lists (times_studied, velocity, in-flight attempts). Two are deliberately not here, both for the same reason — they'd measure nothing as specified:

flashcard review state — no concept-level data exists to read (above);
tutor recency — messages has no user_id, so it needs a sessions → messages join plus JSONB concept matching, which is not "one query" like the others.

Verification

Hermetic 2219 passed / 9 skipped, ruff clean, oracles 0 findings, integration 70 passed, Playwright 47 passed (the one failure is #566, fixed by #568). CI green.

…pports (#556) (#592) * feat(quiz): land the two deferred H4 signals at the scope the data supports (#556) #575 shipped three of the five H4 personalization signals and deferred the other two because, as the issue specified them, both would have measured nothing: * flashcards have no concept link — `topic` is free text every writer sets to the COURSE name — so a concept-name match is a permanent zero; * tutor recency needs a sessions -> messages join (`messages` has no `user_id`) plus JSONB concept matching. Both now land: the first at COURSE scope rather than pretending to concept scope, the second as one bounded owner-scoped `sessions` read plus one `messages` read over the ids it returns. Five new fields, five new F6 dimensions, so the morning can price each signal separately before deciding what stays in the prompt — the issue's stated condition for landing any of them. Both key on the OFFERING while the route holds the abstract `course_id`; `services/academics.py` bridges it once for both, and an empty resolution is reported through `tool_signals.report_empty_result` — a student with graph nodes in a course but no offering of it has every offering-keyed input dark, which is the #553 keyspace shape, not "nothing yet". No migrations. No encrypted column is selected (`flashcards.front`/`back`, `messages.content`). Every read is bounded, owner-scoped, and degrades to unknown rather than to zero. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(quiz): share the course lookups the H4 signals duplicated (#556 review) Review round 1 on the two new H4 signals. Six new round-trips per generation became three, plus four smaller fixes. Three of the six re-asked questions a CONCURRENT leg of the same request was already asking, so nothing could be shared by accident: * `user_offering_ids_for_course` (two UNCACHED reads) — `exam_proximity` wants it too; * the `courses` row — grounding reads `course_code` off it, the flashcard signal reads `course_name` off the same row. `_quiz_via_agent` now resolves both up front (concurrently with each other) and injects them: `CourseRow` into `_course_material`/`_resolve_bu_code`, `offering_ids` into `days_until_next_exam`, and a `CourseScope` into `gather_signals`. Every injection point is an OPTIONAL argument, so existing callers resolve exactly as before. M2: the F5 probe was a tautology — `HAS_GRAPH` scoped to the course asks what the route answered by reading the node on the way in. `report_empty_result` gains an optional `plausible` for callers that hold the fact, which skips the probe read while keeping the `quiz.tool_empty` signal and its expectation. M3: the flashcard read now narrows to one course in PostgREST via an `or=` tree (`offering_id.in.(…)`, `topic.ilike."<course>"`) instead of scanning the student's whole collection and filtering in Python. The count comes from Content-Range, so it is exact no matter what the cap does, and the newest- review ordering keeps the recency answer correct on a truncated page; only the reviewed tally goes unknown there. M5: `_days_since` parses through `routes.quiz._parse_ts` rather than keeping a second copy of the naive/aware rule. M6: a test pins that the ordinary empty path — scope resolves, both reads empty — reports NOTHING. Zero flashcards is what a first-week student looks like, and firing there is the alarm fatigue `tool_signals` exists to avoid. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(quiz): close the three gaps the H4 read-sharing opened (#556 review 2) A — the hoisted resolution had lost the gather's exception backstop. It ran under a bare `asyncio.gather`, so a raise from the to_thread machinery (the one failure `_course_row`/`offering_scope` cannot swallow themselves) escaped `_quiz_via_agent` and 502'd QUIZ_GENERATION_FAILED instead of degrading. `return_exceptions=True` plus the two guards the sibling gather already uses: a failed course lookup becomes `CourseRow(failed=True)` (so E8 still reports coverage_unknown rather than "no BU code"), a failed resolution becomes None (not [], which would assert no enrollment and trip the F5 dark-scope report on a transport failure). B — a failed offering resolution reported a partial flashcard count as a fact. Round 1 made `_course_scope` always return a CourseScope, and the call site no longer gated on it: `offering_ids=None` plus a known course name built a topic-only `or=` tree and counted the generated decks while silently omitting every imported one. Both signals are now gated on `offering_ids is not None` — the situation the module's contract calls unknown. C — `%` and `_` in a course name are LIKE wildcards, and `\` is LIKE's escape character, so `topic.ilike."Math_101"` also matched "Math-101". `_like_literal` escapes all three BEFORE `_pg_quote` doubles the backslashes for PostgREST; the other order emits a bare `\%` that PostgREST unescapes back into a live wildcard. Each fix has a test verified to fail without it (502; a partial count of 1 reported as a fact; the unescaped pattern). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(quiz): read the H4 signals in the keyspace their tables are written in (#556) Merge-gate review of #592. Both new signals resolved their offering scope with `user_offering_ids_for_course` — ENROLLMENT-derived — while every writer of the tables they read stamps an enrollment-agnostic `resolve_offering(course_id, ...)`: * sessions -> resolve_offering(course_id, create=True) (learn.py:431,:882) * flashcards (import) -> resolve_offering(course_id) (flashcards.py:419) * flashcards (generate)-> no offering_id at all, only a free-text topic Neither table has a `course_id` column to fall back on (0025 recreated both without one). After a term rollover the two keyspaces diverge permanently — the no-retake rule keeps the enrollment in the old term while new sessions and imports land on the current term's offering — so an engaged student read 0 sessions AS A FACT, with no F5 report because the offering list was not empty, just wrong. That is the #553/#529 shape this module's own docstring cites. `course_offering_ids(course_id)` now returns EVERY offering of the abstract course: the exact closure of what those writers can stamp, one read instead of two, and no weaker on ownership (both tables carry `user_id` and both reads filter on it). exam_proximity takes the wider set safely — it narrows back to enrollments itself, because `assignments` is enrollment-keyed. Also, the degraded paths that presented an undercount as a verified fact: * flashcards are SKIPPED when the `courses` read RAISED (new `CourseScope.name_failed`), instead of running an offering-only tree that cannot see an AI-generated card; the route's WARNING now says which half it lost rather than claiming all of them * the topic clause is a SUBSTRING match, matching what Study.tsx counts under a course; equality missed every imported deck whose topic is not verbatim the course name (it is a text box the student types into) * the session window is "started in the last 14d OR never ended", so the resume flow's long-lived sessions stop reading as no tutoring — and the prompt line says "or still open" rather than asserting the window * `course_offering_ids` logs a real DB failure at WARNING, not debug, and reports a truncated offering read as unknown rather than a partial list * `plausible=True` is no longer hard-coded into the F5 dark-scope report: the route asserts the graph row it read, `scripts/benchmark_quiz.py` (fixture user, no graph) asserts nothing and the probe decides, and the probe is scoped to the course * `_days_since` counts CALENDAR days, agreeing with the exam line one sentence away instead of disagreeing for most of every day Cleanups the review asked for: `pg_quote_value`/`like_literal` move to db/connection.py as PostgREST grammar and fix routes/onboarding.py's course search, which shipped the bug they exist to prevent; `parse_ts` moves to services/timestamps.py so a service no longer imports a route; the dead, divergent `_course_scope` is gone and `scope` is a required argument; `exam_prompt_line` is extracted so the prompt-budget benchmark measures the real sentence instead of a hand-copy that was two features stale; the budget doc's NULL/zero pricing advice is corrected (it was inverted on both halves). Refs #556, #537. Follow-up filed as #596 (node_mastery_events as the primary tutor-recency read). * refactor(academics): move course_offering_ids to its CLAUDE.md home (#556) Re-review of the #592 merge-gate rework: `course_offering_ids` is term/offering resolution reading `table("course_offerings")`, and CLAUDE.md names `services/academics.py` as the single home for that — a feature module was the wrong place for it, and `exam_proximity` had ended up pointing at `quiz_signals` for an offering concept. Moved verbatim (WARNING + exc_info logging and the scan cap came with it), next to `user_offering_ids_for_course` so the two keyspaces are readable side by side, with each docstring naming when the other is the right one. `routes/quiz.py` imports it from academics now; `quiz_signals` no longer resolves offerings at all — it consumes the injected `CourseScope`. Its unit tests moved to `tests/test_academics.py` with them. `user_offering_ids_for_course` does NOT reuse it, and that is deliberate rather than an omission. Sharing the read means sharing `select_with_count`, and the two want different failure semantics: the counted form never raises and answers `None` for "couldn't tell", while the enrollment form has no tri-state and must let a failed read raise, because degrading to `[]` there would assert the student is enrolled in nothing. Routing one through the other either swallows that or forces the counted form to raise — and in practice the swap also broke 26 tests across gradebook/graph/study-guide that mock only `.select`, plus it would have turned a silently-truncated read into an exception inside `enrollment_id_for`. One duplicated `select` line is the cheaper trade; both docstrings say so, and a new test pins the raising behaviour so a future merge cannot quietly erase it. Two wording fixes the re-review also caught: * `_course_row`'s own WARNING is the one that fires in the common failure path, so it now names both consumers of that row — grounding coverage AND the flashcard signals — instead of grounding alone; * `TestSignalCourseKeyspace::test_the_scope_never_consults_enrollments` overstated what it checked (it asserts the `course_offerings` read happened; it cannot assert `enrollments` was untouched, because the exam leg reads it legitimately). Renamed to `test_the_scope_is_resolved_from_the_courses_own_offerings` and pointed at the module-level twin in `tests/test_academics.py`, which does assert absence properly. Refs #556, #537. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

AndresL230 · 2026-08-22T10:14:46Z

Closes#556 (four of five — see Scope below). Workstream H4 of epic #537.

Each of these was already sitting in a table the generation path touches, and none of it reached the prompt.

signal	cost
`graph_nodes.times_studied`	zero queries — already in the row `generate_quiz` reads to resolve the concept name and course, just never selected
learning velocity	one bounded read; `graph_service._compute_velocity`imported, not re-derived
unfinished attempts on this concept	one owner-scoped read, reusing #542's derived-status rule
flashcards for this concept	one read, a count — front/back are encrypted (#518) and neither read nor needed

times_studied is passed into the gatherer rather than re-read: a second query would spend a round-trip to learn something we were already told.

Landed behind the measurement, as the issue requires

Every signal records its own dimension on quiz.started, which shares a request_id with the llm_usage row — so "is this worth its tokens?" is answerable with data rather than taste. That was the issue's stated condition for landing them at all.

It runs as a fourth leg of the gather that already resolves grounding, the recently-asked list and exam proximity — best-effort context exactly like those three, so it adds no latency. And it never raises: a signal nobody strictly needs must not be able to fail a generation.

Two distinctions the code keeps deliberately

None (couldn't tell) is not 0 (a fact about the student). Collapsing them would report "never studied" for a broken query.
A zero velocity is not reported, because _compute_velocity returns 0.0 both for "no recent gain" and "not enough data" — claiming stagnation we can't distinguish from silence. A zero times_studiedis reported: that one is a fact, and exactly what a generator should know.

Scope — one item deliberately left out

Tutor recency via messages.graph_update_json is not here. The issue frames all five as "one query or one extra selected column", and that one isn't: messages has no user_id (it's session-scoped), so it needs a sessions → messages join plus JSONB concept-name matching through PostgREST. That's a different size of change with a different cost profile, and bolting it on would have made the F6 numbers for the other four harder to read.

Left out deliberately rather than done badly. Flagged on the issue so it doesn't evaporate.

Verification

Hermetic 2218 passed / 9 skipped, ruff clean, oracles 0 findings, integration 70 passed, Playwright 47 passed (the one failure is #566, fixed by #568).

🤖 Generated with Claude Code

Four of the five blind spots the issue lists. Each was already sitting in a table the generation path touches, and none of it reached the prompt: * `graph_nodes.times_studied` — in the row `generate_quiz` ALREADY reads to resolve the concept name and course, simply never selected. One more column, zero extra queries; passed INTO the gatherer rather than re-read, because a second read would spend a round-trip to learn something we were already told. * learning velocity — `graph_service._compute_velocity` is imported, not re-derived. #557 is what two copies of one number costs. * unfinished attempts on this concept — trivially available since #542 gave attempts a derived status; the same `completed_at IS NULL` rule, not a second definition of "unfinished". * flashcards for this concept — a COUNT. Front/back are encrypted (#518) and are neither read nor needed. Landed BEHIND the F6 measurement, which is the condition the issue sets: every signal records its own prompt dimension on `quiz.started`, which shares a `request_id` with the `llm_usage` row, so "is this worth its tokens?" is answerable with data rather than taste. Runs as a fourth leg of the gather that already resolves grounding, the recently-asked list and exam proximity — best-effort context exactly like those three, so it costs no added latency. Never raises: a signal nobody strictly needs must not be able to fail a generation. Two distinctions the code keeps deliberately: * `None` (could not tell) is not `0` (a fact about the student). Collapsing them would report "never studied" for a broken query. * a zero VELOCITY is not reported, because `_compute_velocity` returns 0.0 both for "no recent gain" and "not enough data" — claiming stagnation we cannot distinguish from silence. A zero `times_studied` IS reported: that one is a fact, and exactly what a generator should know. NOT included: tutor recency via `messages.graph_update_json`. The issue frames all five as "one query or one extra selected column", and that one isn't: `messages` has no `user_id` (it is session-scoped), so it needs a sessions -> messages join plus JSONB concept-name matching through PostgREST. Left out deliberately rather than done badly; noted on the issue. Hermetic 2218 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

cloudflare-workers-and-pages · 2026-08-22T10:14:51Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	frontend-staging	`ee1c91e`	Commit Preview URL Branch Preview URL	Aug 22 2026, 10:23 AM

coderabbitai · 2026-08-22T10:14:58Z

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:19 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 @coderabbitai review or push new commits to the PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f78995c1-90ee-4268-a513-4ab94cab82bd

📥 Commits

Reviewing files that changed from the base of the PR and between b14fc3b and ee1c91e.

📒 Files selected for processing (4)

backend/routes/quiz.py
backend/services/quiz_signals.py
backend/tests/test_event_capture_seams.py
backend/tests/test_quiz_signals.py

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

supabase · 2026-08-22T10:17:40Z

This pull request has been ignored for the connected project ybgqdonkoqftwrmweuyv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

Five findings. The first is the one this module was written to avoid. **The velocity was inflated up to 14x, and disagreed with the Tree.** `_compute_velocity` derives its window from `recent[0]` as the OLDEST recent event — which is why `get_graph` feeds it `created_at.asc`. I read `desc` (correct, so the LIMIT keeps the NEWEST events) and handed the rows straight over, collapsing `days` to `max(1, 0) == 1`. Four events of +0.1 over ten days: 0.04 on the graph screen, 0.4 in the quiz prompt. So I reintroduced the exact "two copies of one number" failure the docstring cites #557 for — not by copying the algorithm, but by breaking its contract through the argument. Rows are reversed now, and a test pins the two against each other rather than against a literal. **The flashcard signal is removed, not fixed.** It matched `flashcards.topic` against a concept name, and nothing ever writes a concept name there: every writer stores `body.topic`, and the only caller (`Study.tsx`) passes `course.course_name`. So it was a permanent zero plus a wasted request-path round-trip — and worse, it would have had the F6 measurement conclude the signal is worthless when it was simply never wired to real data, which defeats the entire point of landing these behind a measurement. It needs a concept<->card mapping first; noted in the module and on the issue. **In-flight now shares #542's TTL.** Checking only `completed_at`/ `abandoned_at` IS NULL is a second, looser definition of "unfinished": `_attempt_status` treats an in-progress row past the 24h TTL as abandoned even before the lazy sweep stamps it, and that sweep runs on the history reads, not on generation. A student who generated three quizzes last week and never opened their history would have been told they had three in flight. **Counts are exact, not saturated.** `len(rows)` under a LIMIT reported the cap as though it were a fact; `select_with_count` gives the true total for the same round-trip. **The H3 comment was orphaned** — the H4 block had been inserted between the exam-proximity rationale and the code it explains. Moved below it. Also value-checked the new dimensions in the `quiz.started` pin, which had been comparing the payload against itself and so asserted presence but never correctness. Hermetic 2219 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

AndresL230 · 2026-08-22T10:26:32Z

Review round — five findings, and the first one is the irony of this PR

I broke _compute_velocity by reusing it. The module docstring cites #557 for why the 14-day computation is imported rather than copied — and then I fed it rows in the opposite order from its only other caller. _compute_velocity derives its window from recent[0] as the oldest recent event, which is why get_graph reads created_at.asc. I read desc (correct, so the LIMIT keeps the newest events) and handed them straight over, collapsing days to max(1, 0) == 1.

Four events of +0.1 over ten days:

surface	velocity
Tree / graph screen	0.04
quiz prompt (mine)	0.4

Same concept, two numbers — the exact failure #557 spent a workstream undoing, reintroduced through the argument instead of the algorithm. Rows are reversed now, and the test pins the two computations against each other rather than against a literal.

The flashcard signal is removed, not fixed. It matched flashcards.topic against a concept name, and nothing ever writes a concept name there: routes/flashcards.py stores body.topic, and the only caller (Study.tsx:711) passes course.course_name. So it was a permanent zero plus a wasted request-path round-trip — and worse, it would have had the F6 measurement conclude the signal is worthless when it was simply never wired to real data. That defeats the entire point of landing these behind a measurement. It needs a concept↔card mapping first; documented in the module and flagged on the issue.

In-flight now shares #542's TTL. Checking only completed_at/abandoned_at IS NULL is a second, looser definition of "unfinished": _attempt_status calls an in-progress row past the 24h TTL abandoned even before the lazy sweep stamps it, and that sweep runs on the history reads — not on generation. A student who generated three quizzes last week and never opened their history would have been told they had three in flight.

Counts are exact, not saturated.len(rows) under a LIMIT reported the cap as though it were a fact about the student; select_with_count gives the true total for the same round-trip.

The H3 comment was orphaned — my H4 block had been inserted between the exam-proximity rationale and the code it explains. Moved below it.

Also value-checked the new dimensions in the quiz.started pin, which the review noted were comparing the payload against itself — asserting presence but never correctness.

Scope, restated

This closes three of the five signals #556 lists (times_studied, velocity, in-flight attempts). Two are deliberately not here, both for the same reason — they'd measure nothing as specified:

flashcard review state — no concept-level data exists to read (above);
tutor recency — messages has no user_id, so it needs a sessions → messages join plus JSONB concept matching, which is not "one query" like the others.

Verification

Hermetic 2219 passed / 9 skipped, ruff clean, oracles 0 findings, integration 70 passed, Playwright 47 passed (the one failure is #566, fixed by #568). CI green.

…pports (#556) (#592) * feat(quiz): land the two deferred H4 signals at the scope the data supports (#556) #575 shipped three of the five H4 personalization signals and deferred the other two because, as the issue specified them, both would have measured nothing: * flashcards have no concept link — `topic` is free text every writer sets to the COURSE name — so a concept-name match is a permanent zero; * tutor recency needs a sessions -> messages join (`messages` has no `user_id`) plus JSONB concept matching. Both now land: the first at COURSE scope rather than pretending to concept scope, the second as one bounded owner-scoped `sessions` read plus one `messages` read over the ids it returns. Five new fields, five new F6 dimensions, so the morning can price each signal separately before deciding what stays in the prompt — the issue's stated condition for landing any of them. Both key on the OFFERING while the route holds the abstract `course_id`; `services/academics.py` bridges it once for both, and an empty resolution is reported through `tool_signals.report_empty_result` — a student with graph nodes in a course but no offering of it has every offering-keyed input dark, which is the #553 keyspace shape, not "nothing yet". No migrations. No encrypted column is selected (`flashcards.front`/`back`, `messages.content`). Every read is bounded, owner-scoped, and degrades to unknown rather than to zero. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(quiz): share the course lookups the H4 signals duplicated (#556 review) Review round 1 on the two new H4 signals. Six new round-trips per generation became three, plus four smaller fixes. Three of the six re-asked questions a CONCURRENT leg of the same request was already asking, so nothing could be shared by accident: * `user_offering_ids_for_course` (two UNCACHED reads) — `exam_proximity` wants it too; * the `courses` row — grounding reads `course_code` off it, the flashcard signal reads `course_name` off the same row. `_quiz_via_agent` now resolves both up front (concurrently with each other) and injects them: `CourseRow` into `_course_material`/`_resolve_bu_code`, `offering_ids` into `days_until_next_exam`, and a `CourseScope` into `gather_signals`. Every injection point is an OPTIONAL argument, so existing callers resolve exactly as before. M2: the F5 probe was a tautology — `HAS_GRAPH` scoped to the course asks what the route answered by reading the node on the way in. `report_empty_result` gains an optional `plausible` for callers that hold the fact, which skips the probe read while keeping the `quiz.tool_empty` signal and its expectation. M3: the flashcard read now narrows to one course in PostgREST via an `or=` tree (`offering_id.in.(…)`, `topic.ilike."<course>"`) instead of scanning the student's whole collection and filtering in Python. The count comes from Content-Range, so it is exact no matter what the cap does, and the newest- review ordering keeps the recency answer correct on a truncated page; only the reviewed tally goes unknown there. M5: `_days_since` parses through `routes.quiz._parse_ts` rather than keeping a second copy of the naive/aware rule. M6: a test pins that the ordinary empty path — scope resolves, both reads empty — reports NOTHING. Zero flashcards is what a first-week student looks like, and firing there is the alarm fatigue `tool_signals` exists to avoid. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(quiz): close the three gaps the H4 read-sharing opened (#556 review 2) A — the hoisted resolution had lost the gather's exception backstop. It ran under a bare `asyncio.gather`, so a raise from the to_thread machinery (the one failure `_course_row`/`offering_scope` cannot swallow themselves) escaped `_quiz_via_agent` and 502'd QUIZ_GENERATION_FAILED instead of degrading. `return_exceptions=True` plus the two guards the sibling gather already uses: a failed course lookup becomes `CourseRow(failed=True)` (so E8 still reports coverage_unknown rather than "no BU code"), a failed resolution becomes None (not [], which would assert no enrollment and trip the F5 dark-scope report on a transport failure). B — a failed offering resolution reported a partial flashcard count as a fact. Round 1 made `_course_scope` always return a CourseScope, and the call site no longer gated on it: `offering_ids=None` plus a known course name built a topic-only `or=` tree and counted the generated decks while silently omitting every imported one. Both signals are now gated on `offering_ids is not None` — the situation the module's contract calls unknown. C — `%` and `_` in a course name are LIKE wildcards, and `\` is LIKE's escape character, so `topic.ilike."Math_101"` also matched "Math-101". `_like_literal` escapes all three BEFORE `_pg_quote` doubles the backslashes for PostgREST; the other order emits a bare `\%` that PostgREST unescapes back into a live wildcard. Each fix has a test verified to fail without it (502; a partial count of 1 reported as a fact; the unescaped pattern). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(quiz): read the H4 signals in the keyspace their tables are written in (#556) Merge-gate review of #592. Both new signals resolved their offering scope with `user_offering_ids_for_course` — ENROLLMENT-derived — while every writer of the tables they read stamps an enrollment-agnostic `resolve_offering(course_id, ...)`: * sessions -> resolve_offering(course_id, create=True) (learn.py:431,:882) * flashcards (import) -> resolve_offering(course_id) (flashcards.py:419) * flashcards (generate)-> no offering_id at all, only a free-text topic Neither table has a `course_id` column to fall back on (0025 recreated both without one). After a term rollover the two keyspaces diverge permanently — the no-retake rule keeps the enrollment in the old term while new sessions and imports land on the current term's offering — so an engaged student read 0 sessions AS A FACT, with no F5 report because the offering list was not empty, just wrong. That is the #553/#529 shape this module's own docstring cites. `course_offering_ids(course_id)` now returns EVERY offering of the abstract course: the exact closure of what those writers can stamp, one read instead of two, and no weaker on ownership (both tables carry `user_id` and both reads filter on it). exam_proximity takes the wider set safely — it narrows back to enrollments itself, because `assignments` is enrollment-keyed. Also, the degraded paths that presented an undercount as a verified fact: * flashcards are SKIPPED when the `courses` read RAISED (new `CourseScope.name_failed`), instead of running an offering-only tree that cannot see an AI-generated card; the route's WARNING now says which half it lost rather than claiming all of them * the topic clause is a SUBSTRING match, matching what Study.tsx counts under a course; equality missed every imported deck whose topic is not verbatim the course name (it is a text box the student types into) * the session window is "started in the last 14d OR never ended", so the resume flow's long-lived sessions stop reading as no tutoring — and the prompt line says "or still open" rather than asserting the window * `course_offering_ids` logs a real DB failure at WARNING, not debug, and reports a truncated offering read as unknown rather than a partial list * `plausible=True` is no longer hard-coded into the F5 dark-scope report: the route asserts the graph row it read, `scripts/benchmark_quiz.py` (fixture user, no graph) asserts nothing and the probe decides, and the probe is scoped to the course * `_days_since` counts CALENDAR days, agreeing with the exam line one sentence away instead of disagreeing for most of every day Cleanups the review asked for: `pg_quote_value`/`like_literal` move to db/connection.py as PostgREST grammar and fix routes/onboarding.py's course search, which shipped the bug they exist to prevent; `parse_ts` moves to services/timestamps.py so a service no longer imports a route; the dead, divergent `_course_scope` is gone and `scope` is a required argument; `exam_prompt_line` is extracted so the prompt-budget benchmark measures the real sentence instead of a hand-copy that was two features stale; the budget doc's NULL/zero pricing advice is corrected (it was inverted on both halves). Refs #556, #537. Follow-up filed as #596 (node_mastery_events as the primary tutor-recency read). * refactor(academics): move course_offering_ids to its CLAUDE.md home (#556) Re-review of the #592 merge-gate rework: `course_offering_ids` is term/offering resolution reading `table("course_offerings")`, and CLAUDE.md names `services/academics.py` as the single home for that — a feature module was the wrong place for it, and `exam_proximity` had ended up pointing at `quiz_signals` for an offering concept. Moved verbatim (WARNING + exc_info logging and the scan cap came with it), next to `user_offering_ids_for_course` so the two keyspaces are readable side by side, with each docstring naming when the other is the right one. `routes/quiz.py` imports it from academics now; `quiz_signals` no longer resolves offerings at all — it consumes the injected `CourseScope`. Its unit tests moved to `tests/test_academics.py` with them. `user_offering_ids_for_course` does NOT reuse it, and that is deliberate rather than an omission. Sharing the read means sharing `select_with_count`, and the two want different failure semantics: the counted form never raises and answers `None` for "couldn't tell", while the enrollment form has no tri-state and must let a failed read raise, because degrading to `[]` there would assert the student is enrolled in nothing. Routing one through the other either swallows that or forces the counted form to raise — and in practice the swap also broke 26 tests across gradebook/graph/study-guide that mock only `.select`, plus it would have turned a silently-truncated read into an exception inside `enrollment_id_for`. One duplicated `select` line is the cheaper trade; both docstrings say so, and a new test pins the raising behaviour so a future merge cannot quietly erase it. Two wording fixes the re-review also caught: * `_course_row`'s own WARNING is the one that fires in the common failure path, so it now names both consumers of that row — grounding coverage AND the flashcard signals — instead of grounding alone; * `TestSignalCourseKeyspace::test_the_scope_never_consults_enrollments` overstated what it checked (it asserts the `course_offerings` read happened; it cannot assert `enrollments` was untouched, because the exam leg reads it legitimately). Renamed to `test_the_scope_is_resolved_from_the_courses_own_offerings` and pointed at the module-level twin in `tests/test_academics.py`, which does assert absence properly. Refs #556, #537. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

AndresL230 · 2026-08-22T10:14:46Z

Closes#556 (four of five — see Scope below). Workstream H4 of epic #537.

Each of these was already sitting in a table the generation path touches, and none of it reached the prompt.

signal	cost
`graph_nodes.times_studied`	zero queries — already in the row `generate_quiz` reads to resolve the concept name and course, just never selected
learning velocity	one bounded read; `graph_service._compute_velocity`imported, not re-derived
unfinished attempts on this concept	one owner-scoped read, reusing #542's derived-status rule
flashcards for this concept	one read, a count — front/back are encrypted (#518) and neither read nor needed

times_studied is passed into the gatherer rather than re-read: a second query would spend a round-trip to learn something we were already told.

Landed behind the measurement, as the issue requires

Every signal records its own dimension on quiz.started, which shares a request_id with the llm_usage row — so "is this worth its tokens?" is answerable with data rather than taste. That was the issue's stated condition for landing them at all.

It runs as a fourth leg of the gather that already resolves grounding, the recently-asked list and exam proximity — best-effort context exactly like those three, so it adds no latency. And it never raises: a signal nobody strictly needs must not be able to fail a generation.

Two distinctions the code keeps deliberately

None (couldn't tell) is not 0 (a fact about the student). Collapsing them would report "never studied" for a broken query.
A zero velocity is not reported, because _compute_velocity returns 0.0 both for "no recent gain" and "not enough data" — claiming stagnation we can't distinguish from silence. A zero times_studiedis reported: that one is a fact, and exactly what a generator should know.

Scope — one item deliberately left out

Tutor recency via messages.graph_update_json is not here. The issue frames all five as "one query or one extra selected column", and that one isn't: messages has no user_id (it's session-scoped), so it needs a sessions → messages join plus JSONB concept-name matching through PostgREST. That's a different size of change with a different cost profile, and bolting it on would have made the F6 numbers for the other four harder to read.

Left out deliberately rather than done badly. Flagged on the issue so it doesn't evaporate.

Verification

Hermetic 2218 passed / 9 skipped, ruff clean, oracles 0 findings, integration 70 passed, Playwright 47 passed (the one failure is #566, fixed by #568).

🤖 Generated with Claude Code

Four of the five blind spots the issue lists. Each was already sitting in a table the generation path touches, and none of it reached the prompt: * `graph_nodes.times_studied` — in the row `generate_quiz` ALREADY reads to resolve the concept name and course, simply never selected. One more column, zero extra queries; passed INTO the gatherer rather than re-read, because a second read would spend a round-trip to learn something we were already told. * learning velocity — `graph_service._compute_velocity` is imported, not re-derived. #557 is what two copies of one number costs. * unfinished attempts on this concept — trivially available since #542 gave attempts a derived status; the same `completed_at IS NULL` rule, not a second definition of "unfinished". * flashcards for this concept — a COUNT. Front/back are encrypted (#518) and are neither read nor needed. Landed BEHIND the F6 measurement, which is the condition the issue sets: every signal records its own prompt dimension on `quiz.started`, which shares a `request_id` with the `llm_usage` row, so "is this worth its tokens?" is answerable with data rather than taste. Runs as a fourth leg of the gather that already resolves grounding, the recently-asked list and exam proximity — best-effort context exactly like those three, so it costs no added latency. Never raises: a signal nobody strictly needs must not be able to fail a generation. Two distinctions the code keeps deliberately: * `None` (could not tell) is not `0` (a fact about the student). Collapsing them would report "never studied" for a broken query. * a zero VELOCITY is not reported, because `_compute_velocity` returns 0.0 both for "no recent gain" and "not enough data" — claiming stagnation we cannot distinguish from silence. A zero `times_studied` IS reported: that one is a fact, and exactly what a generator should know. NOT included: tutor recency via `messages.graph_update_json`. The issue frames all five as "one query or one extra selected column", and that one isn't: `messages` has no `user_id` (it is session-scoped), so it needs a sessions -> messages join plus JSONB concept-name matching through PostgREST. Left out deliberately rather than done badly; noted on the issue. Hermetic 2218 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

cloudflare-workers-and-pages · 2026-08-22T10:14:51Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	frontend-staging	`ee1c91e`	Commit Preview URL Branch Preview URL	Aug 22 2026, 10:23 AM

coderabbitai · 2026-08-22T10:14:58Z

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:19 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 @coderabbitai review or push new commits to the PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f78995c1-90ee-4268-a513-4ab94cab82bd

📥 Commits

Reviewing files that changed from the base of the PR and between b14fc3b and ee1c91e.

📒 Files selected for processing (4)

backend/routes/quiz.py
backend/services/quiz_signals.py
backend/tests/test_event_capture_seams.py
backend/tests/test_quiz_signals.py

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

supabase · 2026-08-22T10:17:40Z

This pull request has been ignored for the connected project ybgqdonkoqftwrmweuyv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

Five findings. The first is the one this module was written to avoid. **The velocity was inflated up to 14x, and disagreed with the Tree.** `_compute_velocity` derives its window from `recent[0]` as the OLDEST recent event — which is why `get_graph` feeds it `created_at.asc`. I read `desc` (correct, so the LIMIT keeps the NEWEST events) and handed the rows straight over, collapsing `days` to `max(1, 0) == 1`. Four events of +0.1 over ten days: 0.04 on the graph screen, 0.4 in the quiz prompt. So I reintroduced the exact "two copies of one number" failure the docstring cites #557 for — not by copying the algorithm, but by breaking its contract through the argument. Rows are reversed now, and a test pins the two against each other rather than against a literal. **The flashcard signal is removed, not fixed.** It matched `flashcards.topic` against a concept name, and nothing ever writes a concept name there: every writer stores `body.topic`, and the only caller (`Study.tsx`) passes `course.course_name`. So it was a permanent zero plus a wasted request-path round-trip — and worse, it would have had the F6 measurement conclude the signal is worthless when it was simply never wired to real data, which defeats the entire point of landing these behind a measurement. It needs a concept<->card mapping first; noted in the module and on the issue. **In-flight now shares #542's TTL.** Checking only `completed_at`/ `abandoned_at` IS NULL is a second, looser definition of "unfinished": `_attempt_status` treats an in-progress row past the 24h TTL as abandoned even before the lazy sweep stamps it, and that sweep runs on the history reads, not on generation. A student who generated three quizzes last week and never opened their history would have been told they had three in flight. **Counts are exact, not saturated.** `len(rows)` under a LIMIT reported the cap as though it were a fact; `select_with_count` gives the true total for the same round-trip. **The H3 comment was orphaned** — the H4 block had been inserted between the exam-proximity rationale and the code it explains. Moved below it. Also value-checked the new dimensions in the `quiz.started` pin, which had been comparing the payload against itself and so asserted presence but never correctness. Hermetic 2219 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

AndresL230 · 2026-08-22T10:26:32Z

Review round — five findings, and the first one is the irony of this PR

I broke _compute_velocity by reusing it. The module docstring cites #557 for why the 14-day computation is imported rather than copied — and then I fed it rows in the opposite order from its only other caller. _compute_velocity derives its window from recent[0] as the oldest recent event, which is why get_graph reads created_at.asc. I read desc (correct, so the LIMIT keeps the newest events) and handed them straight over, collapsing days to max(1, 0) == 1.

Four events of +0.1 over ten days:

surface	velocity
Tree / graph screen	0.04
quiz prompt (mine)	0.4

Same concept, two numbers — the exact failure #557 spent a workstream undoing, reintroduced through the argument instead of the algorithm. Rows are reversed now, and the test pins the two computations against each other rather than against a literal.

The flashcard signal is removed, not fixed. It matched flashcards.topic against a concept name, and nothing ever writes a concept name there: routes/flashcards.py stores body.topic, and the only caller (Study.tsx:711) passes course.course_name. So it was a permanent zero plus a wasted request-path round-trip — and worse, it would have had the F6 measurement conclude the signal is worthless when it was simply never wired to real data. That defeats the entire point of landing these behind a measurement. It needs a concept↔card mapping first; documented in the module and flagged on the issue.

In-flight now shares #542's TTL. Checking only completed_at/abandoned_at IS NULL is a second, looser definition of "unfinished": _attempt_status calls an in-progress row past the 24h TTL abandoned even before the lazy sweep stamps it, and that sweep runs on the history reads — not on generation. A student who generated three quizzes last week and never opened their history would have been told they had three in flight.

Counts are exact, not saturated.len(rows) under a LIMIT reported the cap as though it were a fact about the student; select_with_count gives the true total for the same round-trip.

The H3 comment was orphaned — my H4 block had been inserted between the exam-proximity rationale and the code it explains. Moved below it.

Also value-checked the new dimensions in the quiz.started pin, which the review noted were comparing the payload against itself — asserting presence but never correctness.

Scope, restated

This closes three of the five signals #556 lists (times_studied, velocity, in-flight attempts). Two are deliberately not here, both for the same reason — they'd measure nothing as specified:

flashcard review state — no concept-level data exists to read (above);
tutor recency — messages has no user_id, so it needs a sessions → messages join plus JSONB concept matching, which is not "one query" like the others.

Verification

Hermetic 2219 passed / 9 skipped, ruff clean, oracles 0 findings, integration 70 passed, Playwright 47 passed (the one failure is #566, fixed by #568). CI green.

…pports (#556) (#592) * feat(quiz): land the two deferred H4 signals at the scope the data supports (#556) #575 shipped three of the five H4 personalization signals and deferred the other two because, as the issue specified them, both would have measured nothing: * flashcards have no concept link — `topic` is free text every writer sets to the COURSE name — so a concept-name match is a permanent zero; * tutor recency needs a sessions -> messages join (`messages` has no `user_id`) plus JSONB concept matching. Both now land: the first at COURSE scope rather than pretending to concept scope, the second as one bounded owner-scoped `sessions` read plus one `messages` read over the ids it returns. Five new fields, five new F6 dimensions, so the morning can price each signal separately before deciding what stays in the prompt — the issue's stated condition for landing any of them. Both key on the OFFERING while the route holds the abstract `course_id`; `services/academics.py` bridges it once for both, and an empty resolution is reported through `tool_signals.report_empty_result` — a student with graph nodes in a course but no offering of it has every offering-keyed input dark, which is the #553 keyspace shape, not "nothing yet". No migrations. No encrypted column is selected (`flashcards.front`/`back`, `messages.content`). Every read is bounded, owner-scoped, and degrades to unknown rather than to zero. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(quiz): share the course lookups the H4 signals duplicated (#556 review) Review round 1 on the two new H4 signals. Six new round-trips per generation became three, plus four smaller fixes. Three of the six re-asked questions a CONCURRENT leg of the same request was already asking, so nothing could be shared by accident: * `user_offering_ids_for_course` (two UNCACHED reads) — `exam_proximity` wants it too; * the `courses` row — grounding reads `course_code` off it, the flashcard signal reads `course_name` off the same row. `_quiz_via_agent` now resolves both up front (concurrently with each other) and injects them: `CourseRow` into `_course_material`/`_resolve_bu_code`, `offering_ids` into `days_until_next_exam`, and a `CourseScope` into `gather_signals`. Every injection point is an OPTIONAL argument, so existing callers resolve exactly as before. M2: the F5 probe was a tautology — `HAS_GRAPH` scoped to the course asks what the route answered by reading the node on the way in. `report_empty_result` gains an optional `plausible` for callers that hold the fact, which skips the probe read while keeping the `quiz.tool_empty` signal and its expectation. M3: the flashcard read now narrows to one course in PostgREST via an `or=` tree (`offering_id.in.(…)`, `topic.ilike."<course>"`) instead of scanning the student's whole collection and filtering in Python. The count comes from Content-Range, so it is exact no matter what the cap does, and the newest- review ordering keeps the recency answer correct on a truncated page; only the reviewed tally goes unknown there. M5: `_days_since` parses through `routes.quiz._parse_ts` rather than keeping a second copy of the naive/aware rule. M6: a test pins that the ordinary empty path — scope resolves, both reads empty — reports NOTHING. Zero flashcards is what a first-week student looks like, and firing there is the alarm fatigue `tool_signals` exists to avoid. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(quiz): close the three gaps the H4 read-sharing opened (#556 review 2) A — the hoisted resolution had lost the gather's exception backstop. It ran under a bare `asyncio.gather`, so a raise from the to_thread machinery (the one failure `_course_row`/`offering_scope` cannot swallow themselves) escaped `_quiz_via_agent` and 502'd QUIZ_GENERATION_FAILED instead of degrading. `return_exceptions=True` plus the two guards the sibling gather already uses: a failed course lookup becomes `CourseRow(failed=True)` (so E8 still reports coverage_unknown rather than "no BU code"), a failed resolution becomes None (not [], which would assert no enrollment and trip the F5 dark-scope report on a transport failure). B — a failed offering resolution reported a partial flashcard count as a fact. Round 1 made `_course_scope` always return a CourseScope, and the call site no longer gated on it: `offering_ids=None` plus a known course name built a topic-only `or=` tree and counted the generated decks while silently omitting every imported one. Both signals are now gated on `offering_ids is not None` — the situation the module's contract calls unknown. C — `%` and `_` in a course name are LIKE wildcards, and `\` is LIKE's escape character, so `topic.ilike."Math_101"` also matched "Math-101". `_like_literal` escapes all three BEFORE `_pg_quote` doubles the backslashes for PostgREST; the other order emits a bare `\%` that PostgREST unescapes back into a live wildcard. Each fix has a test verified to fail without it (502; a partial count of 1 reported as a fact; the unescaped pattern). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(quiz): read the H4 signals in the keyspace their tables are written in (#556) Merge-gate review of #592. Both new signals resolved their offering scope with `user_offering_ids_for_course` — ENROLLMENT-derived — while every writer of the tables they read stamps an enrollment-agnostic `resolve_offering(course_id, ...)`: * sessions -> resolve_offering(course_id, create=True) (learn.py:431,:882) * flashcards (import) -> resolve_offering(course_id) (flashcards.py:419) * flashcards (generate)-> no offering_id at all, only a free-text topic Neither table has a `course_id` column to fall back on (0025 recreated both without one). After a term rollover the two keyspaces diverge permanently — the no-retake rule keeps the enrollment in the old term while new sessions and imports land on the current term's offering — so an engaged student read 0 sessions AS A FACT, with no F5 report because the offering list was not empty, just wrong. That is the #553/#529 shape this module's own docstring cites. `course_offering_ids(course_id)` now returns EVERY offering of the abstract course: the exact closure of what those writers can stamp, one read instead of two, and no weaker on ownership (both tables carry `user_id` and both reads filter on it). exam_proximity takes the wider set safely — it narrows back to enrollments itself, because `assignments` is enrollment-keyed. Also, the degraded paths that presented an undercount as a verified fact: * flashcards are SKIPPED when the `courses` read RAISED (new `CourseScope.name_failed`), instead of running an offering-only tree that cannot see an AI-generated card; the route's WARNING now says which half it lost rather than claiming all of them * the topic clause is a SUBSTRING match, matching what Study.tsx counts under a course; equality missed every imported deck whose topic is not verbatim the course name (it is a text box the student types into) * the session window is "started in the last 14d OR never ended", so the resume flow's long-lived sessions stop reading as no tutoring — and the prompt line says "or still open" rather than asserting the window * `course_offering_ids` logs a real DB failure at WARNING, not debug, and reports a truncated offering read as unknown rather than a partial list * `plausible=True` is no longer hard-coded into the F5 dark-scope report: the route asserts the graph row it read, `scripts/benchmark_quiz.py` (fixture user, no graph) asserts nothing and the probe decides, and the probe is scoped to the course * `_days_since` counts CALENDAR days, agreeing with the exam line one sentence away instead of disagreeing for most of every day Cleanups the review asked for: `pg_quote_value`/`like_literal` move to db/connection.py as PostgREST grammar and fix routes/onboarding.py's course search, which shipped the bug they exist to prevent; `parse_ts` moves to services/timestamps.py so a service no longer imports a route; the dead, divergent `_course_scope` is gone and `scope` is a required argument; `exam_prompt_line` is extracted so the prompt-budget benchmark measures the real sentence instead of a hand-copy that was two features stale; the budget doc's NULL/zero pricing advice is corrected (it was inverted on both halves). Refs #556, #537. Follow-up filed as #596 (node_mastery_events as the primary tutor-recency read). * refactor(academics): move course_offering_ids to its CLAUDE.md home (#556) Re-review of the #592 merge-gate rework: `course_offering_ids` is term/offering resolution reading `table("course_offerings")`, and CLAUDE.md names `services/academics.py` as the single home for that — a feature module was the wrong place for it, and `exam_proximity` had ended up pointing at `quiz_signals` for an offering concept. Moved verbatim (WARNING + exc_info logging and the scan cap came with it), next to `user_offering_ids_for_course` so the two keyspaces are readable side by side, with each docstring naming when the other is the right one. `routes/quiz.py` imports it from academics now; `quiz_signals` no longer resolves offerings at all — it consumes the injected `CourseScope`. Its unit tests moved to `tests/test_academics.py` with them. `user_offering_ids_for_course` does NOT reuse it, and that is deliberate rather than an omission. Sharing the read means sharing `select_with_count`, and the two want different failure semantics: the counted form never raises and answers `None` for "couldn't tell", while the enrollment form has no tri-state and must let a failed read raise, because degrading to `[]` there would assert the student is enrolled in nothing. Routing one through the other either swallows that or forces the counted form to raise — and in practice the swap also broke 26 tests across gradebook/graph/study-guide that mock only `.select`, plus it would have turned a silently-truncated read into an exception inside `enrollment_id_for`. One duplicated `select` line is the cheaper trade; both docstrings say so, and a new test pins the raising behaviour so a future merge cannot quietly erase it. Two wording fixes the re-review also caught: * `_course_row`'s own WARNING is the one that fires in the common failure path, so it now names both consumers of that row — grounding coverage AND the flashcard signals — instead of grounding alone; * `TestSignalCourseKeyspace::test_the_scope_never_consults_enrollments` overstated what it checked (it asserts the `course_offerings` read happened; it cannot assert `enrollments` was untouched, because the exam leg reads it legitimately). Renamed to `test_the_scope_is_resolved_from_the_courses_own_offerings` and pointed at the module-level twin in `tests/test_academics.py`, which does assert absence properly. Refs #556, #537. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

AndresL230 · 2026-08-22T10:14:46Z

Closes#556 (four of five — see Scope below). Workstream H4 of epic #537.

Each of these was already sitting in a table the generation path touches, and none of it reached the prompt.

signal	cost
`graph_nodes.times_studied`	zero queries — already in the row `generate_quiz` reads to resolve the concept name and course, just never selected
learning velocity	one bounded read; `graph_service._compute_velocity`imported, not re-derived
unfinished attempts on this concept	one owner-scoped read, reusing #542's derived-status rule
flashcards for this concept	one read, a count — front/back are encrypted (#518) and neither read nor needed

times_studied is passed into the gatherer rather than re-read: a second query would spend a round-trip to learn something we were already told.

Landed behind the measurement, as the issue requires

Every signal records its own dimension on quiz.started, which shares a request_id with the llm_usage row — so "is this worth its tokens?" is answerable with data rather than taste. That was the issue's stated condition for landing them at all.

It runs as a fourth leg of the gather that already resolves grounding, the recently-asked list and exam proximity — best-effort context exactly like those three, so it adds no latency. And it never raises: a signal nobody strictly needs must not be able to fail a generation.

Two distinctions the code keeps deliberately

None (couldn't tell) is not 0 (a fact about the student). Collapsing them would report "never studied" for a broken query.
A zero velocity is not reported, because _compute_velocity returns 0.0 both for "no recent gain" and "not enough data" — claiming stagnation we can't distinguish from silence. A zero times_studiedis reported: that one is a fact, and exactly what a generator should know.

Scope — one item deliberately left out

Tutor recency via messages.graph_update_json is not here. The issue frames all five as "one query or one extra selected column", and that one isn't: messages has no user_id (it's session-scoped), so it needs a sessions → messages join plus JSONB concept-name matching through PostgREST. That's a different size of change with a different cost profile, and bolting it on would have made the F6 numbers for the other four harder to read.

Left out deliberately rather than done badly. Flagged on the issue so it doesn't evaporate.

Verification

Hermetic 2218 passed / 9 skipped, ruff clean, oracles 0 findings, integration 70 passed, Playwright 47 passed (the one failure is #566, fixed by #568).

🤖 Generated with Claude Code

Four of the five blind spots the issue lists. Each was already sitting in a table the generation path touches, and none of it reached the prompt: * `graph_nodes.times_studied` — in the row `generate_quiz` ALREADY reads to resolve the concept name and course, simply never selected. One more column, zero extra queries; passed INTO the gatherer rather than re-read, because a second read would spend a round-trip to learn something we were already told. * learning velocity — `graph_service._compute_velocity` is imported, not re-derived. #557 is what two copies of one number costs. * unfinished attempts on this concept — trivially available since #542 gave attempts a derived status; the same `completed_at IS NULL` rule, not a second definition of "unfinished". * flashcards for this concept — a COUNT. Front/back are encrypted (#518) and are neither read nor needed. Landed BEHIND the F6 measurement, which is the condition the issue sets: every signal records its own prompt dimension on `quiz.started`, which shares a `request_id` with the `llm_usage` row, so "is this worth its tokens?" is answerable with data rather than taste. Runs as a fourth leg of the gather that already resolves grounding, the recently-asked list and exam proximity — best-effort context exactly like those three, so it costs no added latency. Never raises: a signal nobody strictly needs must not be able to fail a generation. Two distinctions the code keeps deliberately: * `None` (could not tell) is not `0` (a fact about the student). Collapsing them would report "never studied" for a broken query. * a zero VELOCITY is not reported, because `_compute_velocity` returns 0.0 both for "no recent gain" and "not enough data" — claiming stagnation we cannot distinguish from silence. A zero `times_studied` IS reported: that one is a fact, and exactly what a generator should know. NOT included: tutor recency via `messages.graph_update_json`. The issue frames all five as "one query or one extra selected column", and that one isn't: `messages` has no `user_id` (it is session-scoped), so it needs a sessions -> messages join plus JSONB concept-name matching through PostgREST. Left out deliberately rather than done badly; noted on the issue. Hermetic 2218 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

cloudflare-workers-and-pages · 2026-08-22T10:14:51Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	frontend-staging	`ee1c91e`	Commit Preview URL Branch Preview URL	Aug 22 2026, 10:23 AM

coderabbitai · 2026-08-22T10:14:58Z

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:19 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 @coderabbitai review or push new commits to the PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f78995c1-90ee-4268-a513-4ab94cab82bd

📥 Commits

Reviewing files that changed from the base of the PR and between b14fc3b and ee1c91e.

📒 Files selected for processing (4)

backend/routes/quiz.py
backend/services/quiz_signals.py
backend/tests/test_event_capture_seams.py
backend/tests/test_quiz_signals.py

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

supabase · 2026-08-22T10:17:40Z

This pull request has been ignored for the connected project ybgqdonkoqftwrmweuyv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

Five findings. The first is the one this module was written to avoid. **The velocity was inflated up to 14x, and disagreed with the Tree.** `_compute_velocity` derives its window from `recent[0]` as the OLDEST recent event — which is why `get_graph` feeds it `created_at.asc`. I read `desc` (correct, so the LIMIT keeps the NEWEST events) and handed the rows straight over, collapsing `days` to `max(1, 0) == 1`. Four events of +0.1 over ten days: 0.04 on the graph screen, 0.4 in the quiz prompt. So I reintroduced the exact "two copies of one number" failure the docstring cites #557 for — not by copying the algorithm, but by breaking its contract through the argument. Rows are reversed now, and a test pins the two against each other rather than against a literal. **The flashcard signal is removed, not fixed.** It matched `flashcards.topic` against a concept name, and nothing ever writes a concept name there: every writer stores `body.topic`, and the only caller (`Study.tsx`) passes `course.course_name`. So it was a permanent zero plus a wasted request-path round-trip — and worse, it would have had the F6 measurement conclude the signal is worthless when it was simply never wired to real data, which defeats the entire point of landing these behind a measurement. It needs a concept<->card mapping first; noted in the module and on the issue. **In-flight now shares #542's TTL.** Checking only `completed_at`/ `abandoned_at` IS NULL is a second, looser definition of "unfinished": `_attempt_status` treats an in-progress row past the 24h TTL as abandoned even before the lazy sweep stamps it, and that sweep runs on the history reads, not on generation. A student who generated three quizzes last week and never opened their history would have been told they had three in flight. **Counts are exact, not saturated.** `len(rows)` under a LIMIT reported the cap as though it were a fact; `select_with_count` gives the true total for the same round-trip. **The H3 comment was orphaned** — the H4 block had been inserted between the exam-proximity rationale and the code it explains. Moved below it. Also value-checked the new dimensions in the `quiz.started` pin, which had been comparing the payload against itself and so asserted presence but never correctness. Hermetic 2219 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

AndresL230 · 2026-08-22T10:26:32Z

Review round — five findings, and the first one is the irony of this PR

I broke _compute_velocity by reusing it. The module docstring cites #557 for why the 14-day computation is imported rather than copied — and then I fed it rows in the opposite order from its only other caller. _compute_velocity derives its window from recent[0] as the oldest recent event, which is why get_graph reads created_at.asc. I read desc (correct, so the LIMIT keeps the newest events) and handed them straight over, collapsing days to max(1, 0) == 1.

Four events of +0.1 over ten days:

surface	velocity
Tree / graph screen	0.04
quiz prompt (mine)	0.4

Same concept, two numbers — the exact failure #557 spent a workstream undoing, reintroduced through the argument instead of the algorithm. Rows are reversed now, and the test pins the two computations against each other rather than against a literal.

The flashcard signal is removed, not fixed. It matched flashcards.topic against a concept name, and nothing ever writes a concept name there: routes/flashcards.py stores body.topic, and the only caller (Study.tsx:711) passes course.course_name. So it was a permanent zero plus a wasted request-path round-trip — and worse, it would have had the F6 measurement conclude the signal is worthless when it was simply never wired to real data. That defeats the entire point of landing these behind a measurement. It needs a concept↔card mapping first; documented in the module and flagged on the issue.

In-flight now shares #542's TTL. Checking only completed_at/abandoned_at IS NULL is a second, looser definition of "unfinished": _attempt_status calls an in-progress row past the 24h TTL abandoned even before the lazy sweep stamps it, and that sweep runs on the history reads — not on generation. A student who generated three quizzes last week and never opened their history would have been told they had three in flight.

Counts are exact, not saturated.len(rows) under a LIMIT reported the cap as though it were a fact about the student; select_with_count gives the true total for the same round-trip.

The H3 comment was orphaned — my H4 block had been inserted between the exam-proximity rationale and the code it explains. Moved below it.

Also value-checked the new dimensions in the quiz.started pin, which the review noted were comparing the payload against itself — asserting presence but never correctness.

Scope, restated

This closes three of the five signals #556 lists (times_studied, velocity, in-flight attempts). Two are deliberately not here, both for the same reason — they'd measure nothing as specified:

flashcard review state — no concept-level data exists to read (above);
tutor recency — messages has no user_id, so it needs a sessions → messages join plus JSONB concept matching, which is not "one query" like the others.

Verification

Hermetic 2219 passed / 9 skipped, ruff clean, oracles 0 findings, integration 70 passed, Playwright 47 passed (the one failure is #566, fixed by #568). CI green.

…pports (#556) (#592) * feat(quiz): land the two deferred H4 signals at the scope the data supports (#556) #575 shipped three of the five H4 personalization signals and deferred the other two because, as the issue specified them, both would have measured nothing: * flashcards have no concept link — `topic` is free text every writer sets to the COURSE name — so a concept-name match is a permanent zero; * tutor recency needs a sessions -> messages join (`messages` has no `user_id`) plus JSONB concept matching. Both now land: the first at COURSE scope rather than pretending to concept scope, the second as one bounded owner-scoped `sessions` read plus one `messages` read over the ids it returns. Five new fields, five new F6 dimensions, so the morning can price each signal separately before deciding what stays in the prompt — the issue's stated condition for landing any of them. Both key on the OFFERING while the route holds the abstract `course_id`; `services/academics.py` bridges it once for both, and an empty resolution is reported through `tool_signals.report_empty_result` — a student with graph nodes in a course but no offering of it has every offering-keyed input dark, which is the #553 keyspace shape, not "nothing yet". No migrations. No encrypted column is selected (`flashcards.front`/`back`, `messages.content`). Every read is bounded, owner-scoped, and degrades to unknown rather than to zero. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(quiz): share the course lookups the H4 signals duplicated (#556 review) Review round 1 on the two new H4 signals. Six new round-trips per generation became three, plus four smaller fixes. Three of the six re-asked questions a CONCURRENT leg of the same request was already asking, so nothing could be shared by accident: * `user_offering_ids_for_course` (two UNCACHED reads) — `exam_proximity` wants it too; * the `courses` row — grounding reads `course_code` off it, the flashcard signal reads `course_name` off the same row. `_quiz_via_agent` now resolves both up front (concurrently with each other) and injects them: `CourseRow` into `_course_material`/`_resolve_bu_code`, `offering_ids` into `days_until_next_exam`, and a `CourseScope` into `gather_signals`. Every injection point is an OPTIONAL argument, so existing callers resolve exactly as before. M2: the F5 probe was a tautology — `HAS_GRAPH` scoped to the course asks what the route answered by reading the node on the way in. `report_empty_result` gains an optional `plausible` for callers that hold the fact, which skips the probe read while keeping the `quiz.tool_empty` signal and its expectation. M3: the flashcard read now narrows to one course in PostgREST via an `or=` tree (`offering_id.in.(…)`, `topic.ilike."<course>"`) instead of scanning the student's whole collection and filtering in Python. The count comes from Content-Range, so it is exact no matter what the cap does, and the newest- review ordering keeps the recency answer correct on a truncated page; only the reviewed tally goes unknown there. M5: `_days_since` parses through `routes.quiz._parse_ts` rather than keeping a second copy of the naive/aware rule. M6: a test pins that the ordinary empty path — scope resolves, both reads empty — reports NOTHING. Zero flashcards is what a first-week student looks like, and firing there is the alarm fatigue `tool_signals` exists to avoid. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(quiz): close the three gaps the H4 read-sharing opened (#556 review 2) A — the hoisted resolution had lost the gather's exception backstop. It ran under a bare `asyncio.gather`, so a raise from the to_thread machinery (the one failure `_course_row`/`offering_scope` cannot swallow themselves) escaped `_quiz_via_agent` and 502'd QUIZ_GENERATION_FAILED instead of degrading. `return_exceptions=True` plus the two guards the sibling gather already uses: a failed course lookup becomes `CourseRow(failed=True)` (so E8 still reports coverage_unknown rather than "no BU code"), a failed resolution becomes None (not [], which would assert no enrollment and trip the F5 dark-scope report on a transport failure). B — a failed offering resolution reported a partial flashcard count as a fact. Round 1 made `_course_scope` always return a CourseScope, and the call site no longer gated on it: `offering_ids=None` plus a known course name built a topic-only `or=` tree and counted the generated decks while silently omitting every imported one. Both signals are now gated on `offering_ids is not None` — the situation the module's contract calls unknown. C — `%` and `_` in a course name are LIKE wildcards, and `\` is LIKE's escape character, so `topic.ilike."Math_101"` also matched "Math-101". `_like_literal` escapes all three BEFORE `_pg_quote` doubles the backslashes for PostgREST; the other order emits a bare `\%` that PostgREST unescapes back into a live wildcard. Each fix has a test verified to fail without it (502; a partial count of 1 reported as a fact; the unescaped pattern). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(quiz): read the H4 signals in the keyspace their tables are written in (#556) Merge-gate review of #592. Both new signals resolved their offering scope with `user_offering_ids_for_course` — ENROLLMENT-derived — while every writer of the tables they read stamps an enrollment-agnostic `resolve_offering(course_id, ...)`: * sessions -> resolve_offering(course_id, create=True) (learn.py:431,:882) * flashcards (import) -> resolve_offering(course_id) (flashcards.py:419) * flashcards (generate)-> no offering_id at all, only a free-text topic Neither table has a `course_id` column to fall back on (0025 recreated both without one). After a term rollover the two keyspaces diverge permanently — the no-retake rule keeps the enrollment in the old term while new sessions and imports land on the current term's offering — so an engaged student read 0 sessions AS A FACT, with no F5 report because the offering list was not empty, just wrong. That is the #553/#529 shape this module's own docstring cites. `course_offering_ids(course_id)` now returns EVERY offering of the abstract course: the exact closure of what those writers can stamp, one read instead of two, and no weaker on ownership (both tables carry `user_id` and both reads filter on it). exam_proximity takes the wider set safely — it narrows back to enrollments itself, because `assignments` is enrollment-keyed. Also, the degraded paths that presented an undercount as a verified fact: * flashcards are SKIPPED when the `courses` read RAISED (new `CourseScope.name_failed`), instead of running an offering-only tree that cannot see an AI-generated card; the route's WARNING now says which half it lost rather than claiming all of them * the topic clause is a SUBSTRING match, matching what Study.tsx counts under a course; equality missed every imported deck whose topic is not verbatim the course name (it is a text box the student types into) * the session window is "started in the last 14d OR never ended", so the resume flow's long-lived sessions stop reading as no tutoring — and the prompt line says "or still open" rather than asserting the window * `course_offering_ids` logs a real DB failure at WARNING, not debug, and reports a truncated offering read as unknown rather than a partial list * `plausible=True` is no longer hard-coded into the F5 dark-scope report: the route asserts the graph row it read, `scripts/benchmark_quiz.py` (fixture user, no graph) asserts nothing and the probe decides, and the probe is scoped to the course * `_days_since` counts CALENDAR days, agreeing with the exam line one sentence away instead of disagreeing for most of every day Cleanups the review asked for: `pg_quote_value`/`like_literal` move to db/connection.py as PostgREST grammar and fix routes/onboarding.py's course search, which shipped the bug they exist to prevent; `parse_ts` moves to services/timestamps.py so a service no longer imports a route; the dead, divergent `_course_scope` is gone and `scope` is a required argument; `exam_prompt_line` is extracted so the prompt-budget benchmark measures the real sentence instead of a hand-copy that was two features stale; the budget doc's NULL/zero pricing advice is corrected (it was inverted on both halves). Refs #556, #537. Follow-up filed as #596 (node_mastery_events as the primary tutor-recency read). * refactor(academics): move course_offering_ids to its CLAUDE.md home (#556) Re-review of the #592 merge-gate rework: `course_offering_ids` is term/offering resolution reading `table("course_offerings")`, and CLAUDE.md names `services/academics.py` as the single home for that — a feature module was the wrong place for it, and `exam_proximity` had ended up pointing at `quiz_signals` for an offering concept. Moved verbatim (WARNING + exc_info logging and the scan cap came with it), next to `user_offering_ids_for_course` so the two keyspaces are readable side by side, with each docstring naming when the other is the right one. `routes/quiz.py` imports it from academics now; `quiz_signals` no longer resolves offerings at all — it consumes the injected `CourseScope`. Its unit tests moved to `tests/test_academics.py` with them. `user_offering_ids_for_course` does NOT reuse it, and that is deliberate rather than an omission. Sharing the read means sharing `select_with_count`, and the two want different failure semantics: the counted form never raises and answers `None` for "couldn't tell", while the enrollment form has no tri-state and must let a failed read raise, because degrading to `[]` there would assert the student is enrolled in nothing. Routing one through the other either swallows that or forces the counted form to raise — and in practice the swap also broke 26 tests across gradebook/graph/study-guide that mock only `.select`, plus it would have turned a silently-truncated read into an exception inside `enrollment_id_for`. One duplicated `select` line is the cheaper trade; both docstrings say so, and a new test pins the raising behaviour so a future merge cannot quietly erase it. Two wording fixes the re-review also caught: * `_course_row`'s own WARNING is the one that fires in the common failure path, so it now names both consumers of that row — grounding coverage AND the flashcard signals — instead of grounding alone; * `TestSignalCourseKeyspace::test_the_scope_never_consults_enrollments` overstated what it checked (it asserts the `course_offerings` read happened; it cannot assert `enrollments` was untouched, because the exam leg reads it legitimately). Renamed to `test_the_scope_is_resolved_from_the_courses_own_offerings` and pointed at the module-level twin in `tests/test_academics.py`, which does assert absence properly. Refs #556, #537. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

AndresL230 · 2026-08-22T10:14:46Z

Closes#556 (four of five — see Scope below). Workstream H4 of epic #537.

Each of these was already sitting in a table the generation path touches, and none of it reached the prompt.

signal	cost
`graph_nodes.times_studied`	zero queries — already in the row `generate_quiz` reads to resolve the concept name and course, just never selected
learning velocity	one bounded read; `graph_service._compute_velocity`imported, not re-derived
unfinished attempts on this concept	one owner-scoped read, reusing #542's derived-status rule
flashcards for this concept	one read, a count — front/back are encrypted (#518) and neither read nor needed

times_studied is passed into the gatherer rather than re-read: a second query would spend a round-trip to learn something we were already told.

Landed behind the measurement, as the issue requires

Every signal records its own dimension on quiz.started, which shares a request_id with the llm_usage row — so "is this worth its tokens?" is answerable with data rather than taste. That was the issue's stated condition for landing them at all.

It runs as a fourth leg of the gather that already resolves grounding, the recently-asked list and exam proximity — best-effort context exactly like those three, so it adds no latency. And it never raises: a signal nobody strictly needs must not be able to fail a generation.

Two distinctions the code keeps deliberately

None (couldn't tell) is not 0 (a fact about the student). Collapsing them would report "never studied" for a broken query.
A zero velocity is not reported, because _compute_velocity returns 0.0 both for "no recent gain" and "not enough data" — claiming stagnation we can't distinguish from silence. A zero times_studiedis reported: that one is a fact, and exactly what a generator should know.

Scope — one item deliberately left out

Tutor recency via messages.graph_update_json is not here. The issue frames all five as "one query or one extra selected column", and that one isn't: messages has no user_id (it's session-scoped), so it needs a sessions → messages join plus JSONB concept-name matching through PostgREST. That's a different size of change with a different cost profile, and bolting it on would have made the F6 numbers for the other four harder to read.

Left out deliberately rather than done badly. Flagged on the issue so it doesn't evaporate.

Verification

Hermetic 2218 passed / 9 skipped, ruff clean, oracles 0 findings, integration 70 passed, Playwright 47 passed (the one failure is #566, fixed by #568).

🤖 Generated with Claude Code

Four of the five blind spots the issue lists. Each was already sitting in a table the generation path touches, and none of it reached the prompt: * `graph_nodes.times_studied` — in the row `generate_quiz` ALREADY reads to resolve the concept name and course, simply never selected. One more column, zero extra queries; passed INTO the gatherer rather than re-read, because a second read would spend a round-trip to learn something we were already told. * learning velocity — `graph_service._compute_velocity` is imported, not re-derived. #557 is what two copies of one number costs. * unfinished attempts on this concept — trivially available since #542 gave attempts a derived status; the same `completed_at IS NULL` rule, not a second definition of "unfinished". * flashcards for this concept — a COUNT. Front/back are encrypted (#518) and are neither read nor needed. Landed BEHIND the F6 measurement, which is the condition the issue sets: every signal records its own prompt dimension on `quiz.started`, which shares a `request_id` with the `llm_usage` row, so "is this worth its tokens?" is answerable with data rather than taste. Runs as a fourth leg of the gather that already resolves grounding, the recently-asked list and exam proximity — best-effort context exactly like those three, so it costs no added latency. Never raises: a signal nobody strictly needs must not be able to fail a generation. Two distinctions the code keeps deliberately: * `None` (could not tell) is not `0` (a fact about the student). Collapsing them would report "never studied" for a broken query. * a zero VELOCITY is not reported, because `_compute_velocity` returns 0.0 both for "no recent gain" and "not enough data" — claiming stagnation we cannot distinguish from silence. A zero `times_studied` IS reported: that one is a fact, and exactly what a generator should know. NOT included: tutor recency via `messages.graph_update_json`. The issue frames all five as "one query or one extra selected column", and that one isn't: `messages` has no `user_id` (it is session-scoped), so it needs a sessions -> messages join plus JSONB concept-name matching through PostgREST. Left out deliberately rather than done badly; noted on the issue. Hermetic 2218 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

cloudflare-workers-and-pages · 2026-08-22T10:14:51Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	frontend-staging	`ee1c91e`	Commit Preview URL Branch Preview URL	Aug 22 2026, 10:23 AM

coderabbitai · 2026-08-22T10:14:58Z

Warning

Review limit reached

@AndresL230, you've reached your PR review limit, so we couldn't start this review.

Next review available in:19 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 @coderabbitai review or push new commits to the PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f78995c1-90ee-4268-a513-4ab94cab82bd

📥 Commits

Reviewing files that changed from the base of the PR and between b14fc3b and ee1c91e.

📒 Files selected for processing (4)

backend/routes/quiz.py
backend/services/quiz_signals.py
backend/tests/test_event_capture_seams.py
backend/tests/test_quiz_signals.py

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

supabase · 2026-08-22T10:17:40Z

This pull request has been ignored for the connected project ybgqdonkoqftwrmweuyv because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

Five findings. The first is the one this module was written to avoid. **The velocity was inflated up to 14x, and disagreed with the Tree.** `_compute_velocity` derives its window from `recent[0]` as the OLDEST recent event — which is why `get_graph` feeds it `created_at.asc`. I read `desc` (correct, so the LIMIT keeps the NEWEST events) and handed the rows straight over, collapsing `days` to `max(1, 0) == 1`. Four events of +0.1 over ten days: 0.04 on the graph screen, 0.4 in the quiz prompt. So I reintroduced the exact "two copies of one number" failure the docstring cites #557 for — not by copying the algorithm, but by breaking its contract through the argument. Rows are reversed now, and a test pins the two against each other rather than against a literal. **The flashcard signal is removed, not fixed.** It matched `flashcards.topic` against a concept name, and nothing ever writes a concept name there: every writer stores `body.topic`, and the only caller (`Study.tsx`) passes `course.course_name`. So it was a permanent zero plus a wasted request-path round-trip — and worse, it would have had the F6 measurement conclude the signal is worthless when it was simply never wired to real data, which defeats the entire point of landing these behind a measurement. It needs a concept<->card mapping first; noted in the module and on the issue. **In-flight now shares #542's TTL.** Checking only `completed_at`/ `abandoned_at` IS NULL is a second, looser definition of "unfinished": `_attempt_status` treats an in-progress row past the 24h TTL as abandoned even before the lazy sweep stamps it, and that sweep runs on the history reads, not on generation. A student who generated three quizzes last week and never opened their history would have been told they had three in flight. **Counts are exact, not saturated.** `len(rows)` under a LIMIT reported the cap as though it were a fact; `select_with_count` gives the true total for the same round-trip. **The H3 comment was orphaned** — the H4 block had been inserted between the exam-proximity rationale and the code it explains. Moved below it. Also value-checked the new dimensions in the `quiz.started` pin, which had been comparing the payload against itself and so asserted presence but never correctness. Hermetic 2219 passed / 9 skipped, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

AndresL230 · 2026-08-22T10:26:32Z

Review round — five findings, and the first one is the irony of this PR

I broke _compute_velocity by reusing it. The module docstring cites #557 for why the 14-day computation is imported rather than copied — and then I fed it rows in the opposite order from its only other caller. _compute_velocity derives its window from recent[0] as the oldest recent event, which is why get_graph reads created_at.asc. I read desc (correct, so the LIMIT keeps the newest events) and handed them straight over, collapsing days to max(1, 0) == 1.

Four events of +0.1 over ten days:

surface	velocity
Tree / graph screen	0.04
quiz prompt (mine)	0.4

Same concept, two numbers — the exact failure #557 spent a workstream undoing, reintroduced through the argument instead of the algorithm. Rows are reversed now, and the test pins the two computations against each other rather than against a literal.

The flashcard signal is removed, not fixed. It matched flashcards.topic against a concept name, and nothing ever writes a concept name there: routes/flashcards.py stores body.topic, and the only caller (Study.tsx:711) passes course.course_name. So it was a permanent zero plus a wasted request-path round-trip — and worse, it would have had the F6 measurement conclude the signal is worthless when it was simply never wired to real data. That defeats the entire point of landing these behind a measurement. It needs a concept↔card mapping first; documented in the module and flagged on the issue.

In-flight now shares #542's TTL. Checking only completed_at/abandoned_at IS NULL is a second, looser definition of "unfinished": _attempt_status calls an in-progress row past the 24h TTL abandoned even before the lazy sweep stamps it, and that sweep runs on the history reads — not on generation. A student who generated three quizzes last week and never opened their history would have been told they had three in flight.

Counts are exact, not saturated.len(rows) under a LIMIT reported the cap as though it were a fact about the student; select_with_count gives the true total for the same round-trip.

The H3 comment was orphaned — my H4 block had been inserted between the exam-proximity rationale and the code it explains. Moved below it.

Also value-checked the new dimensions in the quiz.started pin, which the review noted were comparing the payload against itself — asserting presence but never correctness.

Scope, restated

This closes three of the five signals #556 lists (times_studied, velocity, in-flight attempts). Two are deliberately not here, both for the same reason — they'd measure nothing as specified:

flashcard review state — no concept-level data exists to read (above);
tutor recency — messages has no user_id, so it needs a sessions → messages join plus JSONB concept matching, which is not "one query" like the others.

Verification

Hermetic 2219 passed / 9 skipped, ruff clean, oracles 0 findings, integration 70 passed, Playwright 47 passed (the one failure is #566, fixed by #568). CI green.

…pports (#556) (#592) * feat(quiz): land the two deferred H4 signals at the scope the data supports (#556) #575 shipped three of the five H4 personalization signals and deferred the other two because, as the issue specified them, both would have measured nothing: * flashcards have no concept link — `topic` is free text every writer sets to the COURSE name — so a concept-name match is a permanent zero; * tutor recency needs a sessions -> messages join (`messages` has no `user_id`) plus JSONB concept matching. Both now land: the first at COURSE scope rather than pretending to concept scope, the second as one bounded owner-scoped `sessions` read plus one `messages` read over the ids it returns. Five new fields, five new F6 dimensions, so the morning can price each signal separately before deciding what stays in the prompt — the issue's stated condition for landing any of them. Both key on the OFFERING while the route holds the abstract `course_id`; `services/academics.py` bridges it once for both, and an empty resolution is reported through `tool_signals.report_empty_result` — a student with graph nodes in a course but no offering of it has every offering-keyed input dark, which is the #553 keyspace shape, not "nothing yet". No migrations. No encrypted column is selected (`flashcards.front`/`back`, `messages.content`). Every read is bounded, owner-scoped, and degrades to unknown rather than to zero. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(quiz): share the course lookups the H4 signals duplicated (#556 review) Review round 1 on the two new H4 signals. Six new round-trips per generation became three, plus four smaller fixes. Three of the six re-asked questions a CONCURRENT leg of the same request was already asking, so nothing could be shared by accident: * `user_offering_ids_for_course` (two UNCACHED reads) — `exam_proximity` wants it too; * the `courses` row — grounding reads `course_code` off it, the flashcard signal reads `course_name` off the same row. `_quiz_via_agent` now resolves both up front (concurrently with each other) and injects them: `CourseRow` into `_course_material`/`_resolve_bu_code`, `offering_ids` into `days_until_next_exam`, and a `CourseScope` into `gather_signals`. Every injection point is an OPTIONAL argument, so existing callers resolve exactly as before. M2: the F5 probe was a tautology — `HAS_GRAPH` scoped to the course asks what the route answered by reading the node on the way in. `report_empty_result` gains an optional `plausible` for callers that hold the fact, which skips the probe read while keeping the `quiz.tool_empty` signal and its expectation. M3: the flashcard read now narrows to one course in PostgREST via an `or=` tree (`offering_id.in.(…)`, `topic.ilike."<course>"`) instead of scanning the student's whole collection and filtering in Python. The count comes from Content-Range, so it is exact no matter what the cap does, and the newest- review ordering keeps the recency answer correct on a truncated page; only the reviewed tally goes unknown there. M5: `_days_since` parses through `routes.quiz._parse_ts` rather than keeping a second copy of the naive/aware rule. M6: a test pins that the ordinary empty path — scope resolves, both reads empty — reports NOTHING. Zero flashcards is what a first-week student looks like, and firing there is the alarm fatigue `tool_signals` exists to avoid. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(quiz): close the three gaps the H4 read-sharing opened (#556 review 2) A — the hoisted resolution had lost the gather's exception backstop. It ran under a bare `asyncio.gather`, so a raise from the to_thread machinery (the one failure `_course_row`/`offering_scope` cannot swallow themselves) escaped `_quiz_via_agent` and 502'd QUIZ_GENERATION_FAILED instead of degrading. `return_exceptions=True` plus the two guards the sibling gather already uses: a failed course lookup becomes `CourseRow(failed=True)` (so E8 still reports coverage_unknown rather than "no BU code"), a failed resolution becomes None (not [], which would assert no enrollment and trip the F5 dark-scope report on a transport failure). B — a failed offering resolution reported a partial flashcard count as a fact. Round 1 made `_course_scope` always return a CourseScope, and the call site no longer gated on it: `offering_ids=None` plus a known course name built a topic-only `or=` tree and counted the generated decks while silently omitting every imported one. Both signals are now gated on `offering_ids is not None` — the situation the module's contract calls unknown. C — `%` and `_` in a course name are LIKE wildcards, and `\` is LIKE's escape character, so `topic.ilike."Math_101"` also matched "Math-101". `_like_literal` escapes all three BEFORE `_pg_quote` doubles the backslashes for PostgREST; the other order emits a bare `\%` that PostgREST unescapes back into a live wildcard. Each fix has a test verified to fail without it (502; a partial count of 1 reported as a fact; the unescaped pattern). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(quiz): read the H4 signals in the keyspace their tables are written in (#556) Merge-gate review of #592. Both new signals resolved their offering scope with `user_offering_ids_for_course` — ENROLLMENT-derived — while every writer of the tables they read stamps an enrollment-agnostic `resolve_offering(course_id, ...)`: * sessions -> resolve_offering(course_id, create=True) (learn.py:431,:882) * flashcards (import) -> resolve_offering(course_id) (flashcards.py:419) * flashcards (generate)-> no offering_id at all, only a free-text topic Neither table has a `course_id` column to fall back on (0025 recreated both without one). After a term rollover the two keyspaces diverge permanently — the no-retake rule keeps the enrollment in the old term while new sessions and imports land on the current term's offering — so an engaged student read 0 sessions AS A FACT, with no F5 report because the offering list was not empty, just wrong. That is the #553/#529 shape this module's own docstring cites. `course_offering_ids(course_id)` now returns EVERY offering of the abstract course: the exact closure of what those writers can stamp, one read instead of two, and no weaker on ownership (both tables carry `user_id` and both reads filter on it). exam_proximity takes the wider set safely — it narrows back to enrollments itself, because `assignments` is enrollment-keyed. Also, the degraded paths that presented an undercount as a verified fact: * flashcards are SKIPPED when the `courses` read RAISED (new `CourseScope.name_failed`), instead of running an offering-only tree that cannot see an AI-generated card; the route's WARNING now says which half it lost rather than claiming all of them * the topic clause is a SUBSTRING match, matching what Study.tsx counts under a course; equality missed every imported deck whose topic is not verbatim the course name (it is a text box the student types into) * the session window is "started in the last 14d OR never ended", so the resume flow's long-lived sessions stop reading as no tutoring — and the prompt line says "or still open" rather than asserting the window * `course_offering_ids` logs a real DB failure at WARNING, not debug, and reports a truncated offering read as unknown rather than a partial list * `plausible=True` is no longer hard-coded into the F5 dark-scope report: the route asserts the graph row it read, `scripts/benchmark_quiz.py` (fixture user, no graph) asserts nothing and the probe decides, and the probe is scoped to the course * `_days_since` counts CALENDAR days, agreeing with the exam line one sentence away instead of disagreeing for most of every day Cleanups the review asked for: `pg_quote_value`/`like_literal` move to db/connection.py as PostgREST grammar and fix routes/onboarding.py's course search, which shipped the bug they exist to prevent; `parse_ts` moves to services/timestamps.py so a service no longer imports a route; the dead, divergent `_course_scope` is gone and `scope` is a required argument; `exam_prompt_line` is extracted so the prompt-budget benchmark measures the real sentence instead of a hand-copy that was two features stale; the budget doc's NULL/zero pricing advice is corrected (it was inverted on both halves). Refs #556, #537. Follow-up filed as #596 (node_mastery_events as the primary tutor-recency read). * refactor(academics): move course_offering_ids to its CLAUDE.md home (#556) Re-review of the #592 merge-gate rework: `course_offering_ids` is term/offering resolution reading `table("course_offerings")`, and CLAUDE.md names `services/academics.py` as the single home for that — a feature module was the wrong place for it, and `exam_proximity` had ended up pointing at `quiz_signals` for an offering concept. Moved verbatim (WARNING + exc_info logging and the scan cap came with it), next to `user_offering_ids_for_course` so the two keyspaces are readable side by side, with each docstring naming when the other is the right one. `routes/quiz.py` imports it from academics now; `quiz_signals` no longer resolves offerings at all — it consumes the injected `CourseScope`. Its unit tests moved to `tests/test_academics.py` with them. `user_offering_ids_for_course` does NOT reuse it, and that is deliberate rather than an omission. Sharing the read means sharing `select_with_count`, and the two want different failure semantics: the counted form never raises and answers `None` for "couldn't tell", while the enrollment form has no tri-state and must let a failed read raise, because degrading to `[]` there would assert the student is enrolled in nothing. Routing one through the other either swallows that or forces the counted form to raise — and in practice the swap also broke 26 tests across gradebook/graph/study-guide that mock only `.select`, plus it would have turned a silently-truncated read into an exception inside `enrollment_id_for`. One duplicated `select` line is the cheaper trade; both docstrings say so, and a new test pins the raising behaviour so a future merge cannot quietly erase it. Two wording fixes the re-review also caught: * `_course_row`'s own WARNING is the one that fires in the common failure path, so it now names both consumers of that row — grounding coverage AND the flashcard signals — instead of grounding alone; * `TestSignalCourseKeyspace::test_the_scope_never_consults_enrollments` overstated what it checked (it asserts the `course_offerings` read happened; it cannot assert `enrollments` was untouched, because the exam leg reads it legitimately). Renamed to `test_the_scope_is_resolved_from_the_courses_own_offerings` and pointed at the module-level twin in `tests/test_academics.py`, which does assert absence properly. Refs #556, #537. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

AndresL230 merged commit 7863210 into mainAug 22, 2026
7 checks passed

Uh oh!

Conversation

AndresL230 commented Aug 22, 2026

Landed behind the measurement, as the issue requires

Two distinctions the code keeps deliberately

Scope — one item deliberately left out

Verification

Uh oh!

cloudflare-workers-and-pagesBot commented Aug 22, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying with Cloudflare Workers

Uh oh!

coderabbitaiBot commented Aug 22, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

supabaseBot commented Aug 22, 2026

Uh oh!

AndresL230 commented Aug 22, 2026

Review round — five findings, and the first one is the irony of this PR

Scope, restated

Verification

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

AndresL230 commented Aug 22, 2026

Landed behind the measurement, as the issue requires

Two distinctions the code keeps deliberately

Scope — one item deliberately left out

Verification

Uh oh!

cloudflare-workers-and-pagesBot commented Aug 22, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying with Cloudflare Workers

Uh oh!

coderabbitaiBot commented Aug 22, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

supabaseBot commented Aug 22, 2026

Uh oh!

AndresL230 commented Aug 22, 2026

Review round — five findings, and the first one is the irony of this PR

Scope, restated

Verification

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

AndresL230 commented Aug 22, 2026

Landed behind the measurement, as the issue requires

Two distinctions the code keeps deliberately

Scope — one item deliberately left out

Verification

Uh oh!

cloudflare-workers-and-pagesBot commented Aug 22, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying with Cloudflare Workers

Uh oh!

coderabbitaiBot commented Aug 22, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

cloudflare-workers-and-pagesBot commented Aug 22, 2026•
edited
Loading

coderabbitaiBot commented Aug 22, 2026•
edited
Loading

cloudflare-workers-and-pagesBot commented Aug 22, 2026•
edited
Loading

coderabbitaiBot commented Aug 22, 2026•
edited
Loading

cloudflare-workers-and-pagesBot commented Aug 22, 2026•
edited
Loading

coderabbitaiBot commented Aug 22, 2026•
edited
Loading

cloudflare-workers-and-pagesBot commented Aug 22, 2026•
edited
Loading

coderabbitaiBot commented Aug 22, 2026•
edited
Loading