Skip to content

feat(quiz): mine answers_json into the digest as a mistake profile (#554) - #571

Merged
AndresL230 merged 3 commits into
mainfrom
feat/554-distractor-profile-digest
Aug 22, 2026
Merged

feat(quiz): mine answers_json into the digest as a mistake profile (#554)#571
AndresL230 merged 3 commits into
mainfrom
feat/554-distractor-profile-digest

Conversation

@AndresL230

Copy link
Copy Markdown
Collaborator

Closes#554. Workstream H2 of epic #537.

The gold that was never read

quiz_attempts.answers_json has recorded which distractor a student picked, per question, on every submit since the column existed. Nothing has ever read it back.

What the post-submit digest agent actually received was resultslabels only:

{ "question_id": "3", "selected": "B", "correct": false, "correct_answer": "C" }

"Picked B, the answer was C" is not something a model can turn into a misconception. It can only guess one. The option text — the thing that makes a wrong answer mean something — was one join away in questions_json the whole time.

The fix

services/quiz_distractors.py does that join and hands the digest the wrong answers in words: the stem, the concept, what the student chose, and what was correct.

Deliberately dumb — no model call, no I/O, pure data — and it never raises, because it runs inside the post-submit BackgroundTask after the attempt is already graded and written. A crash there would cost the student the digest for a quiz they had already finished.

Per the issue: the join happens in the post-submit task, not at generation, so generation pays nothing for it.

Four things it deliberately does not report

Each would teach the digest something false:

casewhy it's excluded
correct answersa "mistake profile" made of right answers spends tokens reporting the absence of a problem
unanswered questionsskipped is not wrong — recording a blank as a distractor choice invents a misconception out of silence
items with no correct option (#129's shape, which grade wrong for everyone)"the correct answer was nothing" is not a fact about the student
results whose question isn't in the attemptnothing to say

Digest schema version

The other half of the issue. DIGEST_SCHEMA_VERSION is stamped into context_jsonby the model's default rather than by the LLM — a field the model has to remember to set is a field that goes missing — and the reader warns when it meets a version it doesn't understand.

The drift this exists to catch is #548's: the coercer looked for common_errors while the agent wrote common_mistakes, and the symptom was an empty digest, which is indistinguishable from a new student. Unversioned rows are pre-#554 and expected, not a discrepancy.

Tests

Ten, and they pin the wiring as well as the logic — because str.replace on a renamed placeholder is a silent no-op, so the profile would be computed, serialized, and dropped on the floor with nothing to show for it. Also covers the garbage-in cases, since this code's contract is that it cannot throw.

Verification

Hermetic 2181 passed / 9 skipped, ruff clean, oracles 0 findings, integration 56 passed, Playwright 46 passed.

Two Playwright failures, neither from this PR:

🤖 Generated with Claude Code

)
`quiz_attempts.answers_json` has recorded which distractor a student picked,
per question, on every submit since the column existed — and nothing has ever
read it back.
What the post-submit digest agent actually received was `results`, which
carries LABELS: "question 3, picked B, the answer was C". That is not
something a model can turn into a misconception; it can only guess one. The
option TEXT — the thing that makes a wrong answer mean something — was one
join away in `questions_json` the whole time.
`services/quiz_distractors.py` does that join and hands the digest the wrong
answers in words: the stem, the concept, what the student chose, and what was
correct. Deliberately dumb — no model call, no I/O, pure data — and it never
raises, because it runs in the post-submit BackgroundTask after the attempt
is already graded and written, where a crash would cost the student the
digest for a quiz they had already finished.
Four cases it deliberately does NOT report, each of which would teach the
digest something false:
* correct answers — a mistake profile made of right answers spends tokens
to report the absence of a problem;
* unanswered questions — skipped is not wrong, and recording a blank as a
distractor choice invents a misconception out of silence;
* items with no correct option (#129's shape, which grade as wrong for
everyone) — "the correct answer was <nothing>" is not a fact about the
student;
* results whose question is missing from the attempt.
Digest schema version, the other half of the issue: `DIGEST_SCHEMA_VERSION`
is stamped into `context_json` by the model's default rather than by the LLM
(a field the model must remember to set is a field that goes missing), and
the reader warns when it meets a version it doesn't understand. The drift
this exists to catch is #548's: the coercer looked for `common_errors` while
the agent wrote `common_mistakes`, and the symptom was an empty digest —
indistinguishable from a new student.
Tests pin the wiring as well as the logic, because `str.replace` on a
renamed placeholder is a silent no-op: the profile would be computed,
serialized, and dropped on the floor with nothing to show for it.
Hermetic 2181 passed / 9 skipped, ruff clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Aug 22, 2026

Copy link
Copy Markdown

Warning

Review limit reached

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

Next review available in:26 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: d01c32fe-14bd-4aba-9137-f8e87d141887

📥 Commits

Reviewing files that changed from the base of the PR and between c7f043c and bdbcf4f.

📒 Files selected for processing (8)
  • backend/agents/tools/quiz_history.py
  • backend/prompts/quiz_context_update.txt
  • backend/routes/quiz.py
  • backend/services/quiz_context_service.py
  • backend/services/quiz_distractors.py
  • backend/tests/integration/test_quiz_context_repair_db.py
  • backend/tests/test_quiz_context_service.py
  • backend/tests/test_quiz_distractor_profile.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

supabaseBot commented Aug 22, 2026

Copy link
Copy Markdown

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 ↗︎.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Aug 22, 2026

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

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

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend-stagingbdbcf4fCommit Preview URL

Branch Preview URL
Aug 22 2026, 09:03 AM

AndresL230and others added 2 commits August 22, 2026 04:54
Three findings, all about the schema-version half rather than the profile.
**The version was LLM-controlled.** `schema_version` was a field on the
digest agent's output_type, and `submit_quiz` persists `model_dump()`
verbatim. Worse, this PR's own prompt feeds the previous digest back in under
"update your notes" — so from now on the model would SEE `schema_version: 2`
and be invited to update it. A model that helpfully bumped it to 3 would trip
the reader's unknown-shape warning on every later read for that
(user, concept), forever, with no real drift; one that lowered it would kill
the guard just as quietly. The field is now off the agent schema entirely
(so it costs no decoding budget either) and `save_quiz_context` stamps it
server-side, which also covers every future writer.
**The guard couldn't catch the drift it cited.** A version comparison only
fires for a writer NEWER than the reader — a mixed-deploy window. #548 was a
key RENAME at the same version, and catching that with a version number
requires remembering to bump it in the same commit as the rename, which is
precisely the discipline that failed the first time. The version now claims
only what it can prove, and the rename case is caught by its OUTCOME: a row
that is present, stamped with a version this reader understands, and yet
yields nothing readable. A real digest cannot be all three at once — the
agent always writes at least `questions_seen_summary` or `notes` — so that
combination means the keys moved. Logged with the actual key set.
**The docstring understated the blast radius.** It claimed this runs in the
post-submit BackgroundTask. It does not: `submit_quiz` builds the whole
prompt string synchronously and hands only the finished string to
`add_task`. So an escaping exception 500s the submit AFTER the atomic
`completed_at` claim, the mastery write and the score update have landed —
the student sees a failed submit for a quiz that scored, and the retry 409s
with the score never returned. The guard was right; the comment beside it
invited a future reader to delete it as cheap background-task failure.
The review also found a live instance of this same drift class in the OTHER
reader of `context_json`: `course_context_service` harvests
`effective_explanations`, a key no agent has ever written, so that column has
persisted an empty array for every offering since it existed. Out of scope
here, filed as #572.
Hermetic 2184 passed / 9 skipped, ruff clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`test_save_quiz_context_upserts_one_row_and_encrypts` asserts the payload
round-trips exactly, so server-stamping `schema_version` in
`save_quiz_context` changed what comes back. The hermetic twin was updated
with the fix; this is its real-DB counterpart, and only the integration lane
could see it — which is the whole argument for that lane.
Integration 56 passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@AndresL230

Copy link
Copy Markdown
CollaboratorAuthor

Review round — three findings, all fixed

All three were about the schema-version half, not the profile.

1. The version was the model's to change.schema_version was a field on the digest agent's output_type, and submit_quiz persists model_dump() verbatim — so the model owned it. Worse, this PR's own change feeds the previous digest back into the prompt under "update your notes", so from here on the model would seeschema_version: 2 and be invited to update it. A model that helpfully bumped it to 3 would trip the reader's unknown-shape warning on every later read for that (user, concept), forever, with no real drift; one that lowered it would kill the guard just as quietly. The field is off the agent schema entirely now (it also stops costing decoding budget), and save_quiz_context stamps it server-side — which covers every future writer rather than just this one.

2. The guard couldn't catch the drift it cited.version > DIGEST_SCHEMA_VERSION only fires for a writer newer than the reader — a mixed-deploy window. But #548 was a key rename at the same version, and catching that with a version number requires remembering to bump it in the same commit as the rename, which is exactly the discipline that failed the first time. The version now claims only what it can prove, and the rename case is caught by its outcome: a row that is present, stamped with a version this reader understands, and yet yields nothing readable. A real digest can't be all three at once — the agent always writes at least questions_seen_summary or notes — so that combination means the keys moved. Logged with the actual key set, so the next person sees what it drifted to.

3. The docstring understated the blast radius. It said this runs in the post-submit BackgroundTask. It doesn't: submit_quiz builds the whole prompt string synchronously and hands only the finished string to add_task. So an escaping exception 500s the submit after the atomic completed_at claim, the apply_graph_update mastery write and the score update have all landed — the student sees a failed submit for a quiz that scored, and their retry gets a 409 with the score never returned. The guard was right; the comment beside it invited a future reader to delete it as cheap background-task failure.

A live instance of the same bug class, elsewhere

The review also found that course_context_service._parse_quiz_context_to_arrays harvests effective_explanations from context_jsona key no agent has ever written. Its two siblings in the same function read the correct live keys, which is what makes it easy to miss. So offering_concept_stats.effective_explanations has persisted an empty array for every offering since the column existed, indistinguishable from "this class produced none".

I verified it before filing (common_misconceptionscommon_mistakes ✅, prerequisite_gapsweak_areas ✅, effective_explanations ← dead ❌). Out of scope here, filed as #572 — it needs a product decision (make it real vs. delete it) rather than a quiet patch.

The integration lane earned its keep

Server-stamping the version broke test_save_quiz_context_upserts_one_row_and_encrypts, which round-trips the payload against a real database. The hermetic twin was updated alongside the fix; only the real-DB lane caught the second one.

Verification

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

@AndresL230
AndresL230 merged commit e139954 into mainAug 22, 2026
8 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

quiz H2: mine answers_json into the digest (distractor profile) + digest schema version

1 participant

@AndresL230
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
feat(quiz): mine answers_json into the digest as a mistake profile (#554) by AndresL230 · Pull Request #571 · SaplingLearn/Sapling · GitHub
Skip to content

feat(quiz): mine answers_json into the digest as a mistake profile (#554) - #571

Merged
AndresL230 merged 3 commits into
mainfrom
feat/554-distractor-profile-digest
Aug 22, 2026
Merged

feat(quiz): mine answers_json into the digest as a mistake profile (#554)#571
AndresL230 merged 3 commits into
mainfrom
feat/554-distractor-profile-digest

Conversation

@AndresL230

Copy link
Copy Markdown
Collaborator

Closes#554. Workstream H2 of epic #537.

The gold that was never read

quiz_attempts.answers_json has recorded which distractor a student picked, per question, on every submit since the column existed. Nothing has ever read it back.

What the post-submit digest agent actually received was resultslabels only:

{ "question_id": "3", "selected": "B", "correct": false, "correct_answer": "C" }

"Picked B, the answer was C" is not something a model can turn into a misconception. It can only guess one. The option text — the thing that makes a wrong answer mean something — was one join away in questions_json the whole time.

The fix

services/quiz_distractors.py does that join and hands the digest the wrong answers in words: the stem, the concept, what the student chose, and what was correct.

Deliberately dumb — no model call, no I/O, pure data — and it never raises, because it runs inside the post-submit BackgroundTask after the attempt is already graded and written. A crash there would cost the student the digest for a quiz they had already finished.

Per the issue: the join happens in the post-submit task, not at generation, so generation pays nothing for it.

Four things it deliberately does not report

Each would teach the digest something false:

casewhy it's excluded
correct answersa "mistake profile" made of right answers spends tokens reporting the absence of a problem
unanswered questionsskipped is not wrong — recording a blank as a distractor choice invents a misconception out of silence
items with no correct option (#129's shape, which grade wrong for everyone)"the correct answer was nothing" is not a fact about the student
results whose question isn't in the attemptnothing to say

Digest schema version

The other half of the issue. DIGEST_SCHEMA_VERSION is stamped into context_jsonby the model's default rather than by the LLM — a field the model has to remember to set is a field that goes missing — and the reader warns when it meets a version it doesn't understand.

The drift this exists to catch is #548's: the coercer looked for common_errors while the agent wrote common_mistakes, and the symptom was an empty digest, which is indistinguishable from a new student. Unversioned rows are pre-#554 and expected, not a discrepancy.

Tests

Ten, and they pin the wiring as well as the logic — because str.replace on a renamed placeholder is a silent no-op, so the profile would be computed, serialized, and dropped on the floor with nothing to show for it. Also covers the garbage-in cases, since this code's contract is that it cannot throw.

Verification

Hermetic 2181 passed / 9 skipped, ruff clean, oracles 0 findings, integration 56 passed, Playwright 46 passed.

Two Playwright failures, neither from this PR:

🤖 Generated with Claude Code

)
`quiz_attempts.answers_json` has recorded which distractor a student picked,
per question, on every submit since the column existed — and nothing has ever
read it back.
What the post-submit digest agent actually received was `results`, which
carries LABELS: "question 3, picked B, the answer was C". That is not
something a model can turn into a misconception; it can only guess one. The
option TEXT — the thing that makes a wrong answer mean something — was one
join away in `questions_json` the whole time.
`services/quiz_distractors.py` does that join and hands the digest the wrong
answers in words: the stem, the concept, what the student chose, and what was
correct. Deliberately dumb — no model call, no I/O, pure data — and it never
raises, because it runs in the post-submit BackgroundTask after the attempt
is already graded and written, where a crash would cost the student the
digest for a quiz they had already finished.
Four cases it deliberately does NOT report, each of which would teach the
digest something false:
* correct answers — a mistake profile made of right answers spends tokens
to report the absence of a problem;
* unanswered questions — skipped is not wrong, and recording a blank as a
distractor choice invents a misconception out of silence;
* items with no correct option (#129's shape, which grade as wrong for
everyone) — "the correct answer was <nothing>" is not a fact about the
student;
* results whose question is missing from the attempt.
Digest schema version, the other half of the issue: `DIGEST_SCHEMA_VERSION`
is stamped into `context_json` by the model's default rather than by the LLM
(a field the model must remember to set is a field that goes missing), and
the reader warns when it meets a version it doesn't understand. The drift
this exists to catch is #548's: the coercer looked for `common_errors` while
the agent wrote `common_mistakes`, and the symptom was an empty digest —
indistinguishable from a new student.
Tests pin the wiring as well as the logic, because `str.replace` on a
renamed placeholder is a silent no-op: the profile would be computed,
serialized, and dropped on the floor with nothing to show for it.
Hermetic 2181 passed / 9 skipped, ruff clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Aug 22, 2026

Copy link
Copy Markdown

Warning

Review limit reached

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

Next review available in:26 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: d01c32fe-14bd-4aba-9137-f8e87d141887

📥 Commits

Reviewing files that changed from the base of the PR and between c7f043c and bdbcf4f.

📒 Files selected for processing (8)
  • backend/agents/tools/quiz_history.py
  • backend/prompts/quiz_context_update.txt
  • backend/routes/quiz.py
  • backend/services/quiz_context_service.py
  • backend/services/quiz_distractors.py
  • backend/tests/integration/test_quiz_context_repair_db.py
  • backend/tests/test_quiz_context_service.py
  • backend/tests/test_quiz_distractor_profile.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

supabaseBot commented Aug 22, 2026

Copy link
Copy Markdown

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 ↗︎.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Aug 22, 2026

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

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

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend-stagingbdbcf4fCommit Preview URL

Branch Preview URL
Aug 22 2026, 09:03 AM

AndresL230and others added 2 commits August 22, 2026 04:54
Three findings, all about the schema-version half rather than the profile.
**The version was LLM-controlled.** `schema_version` was a field on the
digest agent's output_type, and `submit_quiz` persists `model_dump()`
verbatim. Worse, this PR's own prompt feeds the previous digest back in under
"update your notes" — so from now on the model would SEE `schema_version: 2`
and be invited to update it. A model that helpfully bumped it to 3 would trip
the reader's unknown-shape warning on every later read for that
(user, concept), forever, with no real drift; one that lowered it would kill
the guard just as quietly. The field is now off the agent schema entirely
(so it costs no decoding budget either) and `save_quiz_context` stamps it
server-side, which also covers every future writer.
**The guard couldn't catch the drift it cited.** A version comparison only
fires for a writer NEWER than the reader — a mixed-deploy window. #548 was a
key RENAME at the same version, and catching that with a version number
requires remembering to bump it in the same commit as the rename, which is
precisely the discipline that failed the first time. The version now claims
only what it can prove, and the rename case is caught by its OUTCOME: a row
that is present, stamped with a version this reader understands, and yet
yields nothing readable. A real digest cannot be all three at once — the
agent always writes at least `questions_seen_summary` or `notes` — so that
combination means the keys moved. Logged with the actual key set.
**The docstring understated the blast radius.** It claimed this runs in the
post-submit BackgroundTask. It does not: `submit_quiz` builds the whole
prompt string synchronously and hands only the finished string to
`add_task`. So an escaping exception 500s the submit AFTER the atomic
`completed_at` claim, the mastery write and the score update have landed —
the student sees a failed submit for a quiz that scored, and the retry 409s
with the score never returned. The guard was right; the comment beside it
invited a future reader to delete it as cheap background-task failure.
The review also found a live instance of this same drift class in the OTHER
reader of `context_json`: `course_context_service` harvests
`effective_explanations`, a key no agent has ever written, so that column has
persisted an empty array for every offering since it existed. Out of scope
here, filed as #572.
Hermetic 2184 passed / 9 skipped, ruff clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`test_save_quiz_context_upserts_one_row_and_encrypts` asserts the payload
round-trips exactly, so server-stamping `schema_version` in
`save_quiz_context` changed what comes back. The hermetic twin was updated
with the fix; this is its real-DB counterpart, and only the integration lane
could see it — which is the whole argument for that lane.
Integration 56 passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@AndresL230

Copy link
Copy Markdown
CollaboratorAuthor

Review round — three findings, all fixed

All three were about the schema-version half, not the profile.

1. The version was the model's to change.schema_version was a field on the digest agent's output_type, and submit_quiz persists model_dump() verbatim — so the model owned it. Worse, this PR's own change feeds the previous digest back into the prompt under "update your notes", so from here on the model would seeschema_version: 2 and be invited to update it. A model that helpfully bumped it to 3 would trip the reader's unknown-shape warning on every later read for that (user, concept), forever, with no real drift; one that lowered it would kill the guard just as quietly. The field is off the agent schema entirely now (it also stops costing decoding budget), and save_quiz_context stamps it server-side — which covers every future writer rather than just this one.

2. The guard couldn't catch the drift it cited.version > DIGEST_SCHEMA_VERSION only fires for a writer newer than the reader — a mixed-deploy window. But #548 was a key rename at the same version, and catching that with a version number requires remembering to bump it in the same commit as the rename, which is exactly the discipline that failed the first time. The version now claims only what it can prove, and the rename case is caught by its outcome: a row that is present, stamped with a version this reader understands, and yet yields nothing readable. A real digest can't be all three at once — the agent always writes at least questions_seen_summary or notes — so that combination means the keys moved. Logged with the actual key set, so the next person sees what it drifted to.

3. The docstring understated the blast radius. It said this runs in the post-submit BackgroundTask. It doesn't: submit_quiz builds the whole prompt string synchronously and hands only the finished string to add_task. So an escaping exception 500s the submit after the atomic completed_at claim, the apply_graph_update mastery write and the score update have all landed — the student sees a failed submit for a quiz that scored, and their retry gets a 409 with the score never returned. The guard was right; the comment beside it invited a future reader to delete it as cheap background-task failure.

A live instance of the same bug class, elsewhere

The review also found that course_context_service._parse_quiz_context_to_arrays harvests effective_explanations from context_jsona key no agent has ever written. Its two siblings in the same function read the correct live keys, which is what makes it easy to miss. So offering_concept_stats.effective_explanations has persisted an empty array for every offering since the column existed, indistinguishable from "this class produced none".

I verified it before filing (common_misconceptionscommon_mistakes ✅, prerequisite_gapsweak_areas ✅, effective_explanations ← dead ❌). Out of scope here, filed as #572 — it needs a product decision (make it real vs. delete it) rather than a quiet patch.

The integration lane earned its keep

Server-stamping the version broke test_save_quiz_context_upserts_one_row_and_encrypts, which round-trips the payload against a real database. The hermetic twin was updated alongside the fix; only the real-DB lane caught the second one.

Verification

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

@AndresL230
AndresL230 merged commit e139954 into mainAug 22, 2026
8 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

quiz H2: mine answers_json into the digest (distractor profile) + digest schema version

1 participant

@AndresL230
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' feat(quiz): mine answers_json into the digest as a mistake profile (#554) by AndresL230 · Pull Request #571 · SaplingLearn/Sapling · GitHub
Skip to content

feat(quiz): mine answers_json into the digest as a mistake profile (#554) - #571

Merged
AndresL230 merged 3 commits into
mainfrom
feat/554-distractor-profile-digest
Aug 22, 2026
Merged

feat(quiz): mine answers_json into the digest as a mistake profile (#554)#571
AndresL230 merged 3 commits into
mainfrom
feat/554-distractor-profile-digest

Conversation

@AndresL230

Copy link
Copy Markdown
Collaborator

Closes#554. Workstream H2 of epic #537.

The gold that was never read

quiz_attempts.answers_json has recorded which distractor a student picked, per question, on every submit since the column existed. Nothing has ever read it back.

What the post-submit digest agent actually received was resultslabels only:

{ "question_id": "3", "selected": "B", "correct": false, "correct_answer": "C" }

"Picked B, the answer was C" is not something a model can turn into a misconception. It can only guess one. The option text — the thing that makes a wrong answer mean something — was one join away in questions_json the whole time.

The fix

services/quiz_distractors.py does that join and hands the digest the wrong answers in words: the stem, the concept, what the student chose, and what was correct.

Deliberately dumb — no model call, no I/O, pure data — and it never raises, because it runs inside the post-submit BackgroundTask after the attempt is already graded and written. A crash there would cost the student the digest for a quiz they had already finished.

Per the issue: the join happens in the post-submit task, not at generation, so generation pays nothing for it.

Four things it deliberately does not report

Each would teach the digest something false:

casewhy it's excluded
correct answersa "mistake profile" made of right answers spends tokens reporting the absence of a problem
unanswered questionsskipped is not wrong — recording a blank as a distractor choice invents a misconception out of silence
items with no correct option (#129's shape, which grade wrong for everyone)"the correct answer was nothing" is not a fact about the student
results whose question isn't in the attemptnothing to say

Digest schema version

The other half of the issue. DIGEST_SCHEMA_VERSION is stamped into context_jsonby the model's default rather than by the LLM — a field the model has to remember to set is a field that goes missing — and the reader warns when it meets a version it doesn't understand.

The drift this exists to catch is #548's: the coercer looked for common_errors while the agent wrote common_mistakes, and the symptom was an empty digest, which is indistinguishable from a new student. Unversioned rows are pre-#554 and expected, not a discrepancy.

Tests

Ten, and they pin the wiring as well as the logic — because str.replace on a renamed placeholder is a silent no-op, so the profile would be computed, serialized, and dropped on the floor with nothing to show for it. Also covers the garbage-in cases, since this code's contract is that it cannot throw.

Verification

Hermetic 2181 passed / 9 skipped, ruff clean, oracles 0 findings, integration 56 passed, Playwright 46 passed.

Two Playwright failures, neither from this PR:

🤖 Generated with Claude Code

)
`quiz_attempts.answers_json` has recorded which distractor a student picked,
per question, on every submit since the column existed — and nothing has ever
read it back.
What the post-submit digest agent actually received was `results`, which
carries LABELS: "question 3, picked B, the answer was C". That is not
something a model can turn into a misconception; it can only guess one. The
option TEXT — the thing that makes a wrong answer mean something — was one
join away in `questions_json` the whole time.
`services/quiz_distractors.py` does that join and hands the digest the wrong
answers in words: the stem, the concept, what the student chose, and what was
correct. Deliberately dumb — no model call, no I/O, pure data — and it never
raises, because it runs in the post-submit BackgroundTask after the attempt
is already graded and written, where a crash would cost the student the
digest for a quiz they had already finished.
Four cases it deliberately does NOT report, each of which would teach the
digest something false:
* correct answers — a mistake profile made of right answers spends tokens
to report the absence of a problem;
* unanswered questions — skipped is not wrong, and recording a blank as a
distractor choice invents a misconception out of silence;
* items with no correct option (#129's shape, which grade as wrong for
everyone) — "the correct answer was <nothing>" is not a fact about the
student;
* results whose question is missing from the attempt.
Digest schema version, the other half of the issue: `DIGEST_SCHEMA_VERSION`
is stamped into `context_json` by the model's default rather than by the LLM
(a field the model must remember to set is a field that goes missing), and
the reader warns when it meets a version it doesn't understand. The drift
this exists to catch is #548's: the coercer looked for `common_errors` while
the agent wrote `common_mistakes`, and the symptom was an empty digest —
indistinguishable from a new student.
Tests pin the wiring as well as the logic, because `str.replace` on a
renamed placeholder is a silent no-op: the profile would be computed,
serialized, and dropped on the floor with nothing to show for it.
Hermetic 2181 passed / 9 skipped, ruff clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Aug 22, 2026

Copy link
Copy Markdown

Warning

Review limit reached

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

Next review available in:26 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: d01c32fe-14bd-4aba-9137-f8e87d141887

📥 Commits

Reviewing files that changed from the base of the PR and between c7f043c and bdbcf4f.

📒 Files selected for processing (8)
  • backend/agents/tools/quiz_history.py
  • backend/prompts/quiz_context_update.txt
  • backend/routes/quiz.py
  • backend/services/quiz_context_service.py
  • backend/services/quiz_distractors.py
  • backend/tests/integration/test_quiz_context_repair_db.py
  • backend/tests/test_quiz_context_service.py
  • backend/tests/test_quiz_distractor_profile.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

supabaseBot commented Aug 22, 2026

Copy link
Copy Markdown

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 ↗︎.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Aug 22, 2026

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

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

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend-stagingbdbcf4fCommit Preview URL

Branch Preview URL
Aug 22 2026, 09:03 AM

AndresL230and others added 2 commits August 22, 2026 04:54
Three findings, all about the schema-version half rather than the profile.
**The version was LLM-controlled.** `schema_version` was a field on the
digest agent's output_type, and `submit_quiz` persists `model_dump()`
verbatim. Worse, this PR's own prompt feeds the previous digest back in under
"update your notes" — so from now on the model would SEE `schema_version: 2`
and be invited to update it. A model that helpfully bumped it to 3 would trip
the reader's unknown-shape warning on every later read for that
(user, concept), forever, with no real drift; one that lowered it would kill
the guard just as quietly. The field is now off the agent schema entirely
(so it costs no decoding budget either) and `save_quiz_context` stamps it
server-side, which also covers every future writer.
**The guard couldn't catch the drift it cited.** A version comparison only
fires for a writer NEWER than the reader — a mixed-deploy window. #548 was a
key RENAME at the same version, and catching that with a version number
requires remembering to bump it in the same commit as the rename, which is
precisely the discipline that failed the first time. The version now claims
only what it can prove, and the rename case is caught by its OUTCOME: a row
that is present, stamped with a version this reader understands, and yet
yields nothing readable. A real digest cannot be all three at once — the
agent always writes at least `questions_seen_summary` or `notes` — so that
combination means the keys moved. Logged with the actual key set.
**The docstring understated the blast radius.** It claimed this runs in the
post-submit BackgroundTask. It does not: `submit_quiz` builds the whole
prompt string synchronously and hands only the finished string to
`add_task`. So an escaping exception 500s the submit AFTER the atomic
`completed_at` claim, the mastery write and the score update have landed —
the student sees a failed submit for a quiz that scored, and the retry 409s
with the score never returned. The guard was right; the comment beside it
invited a future reader to delete it as cheap background-task failure.
The review also found a live instance of this same drift class in the OTHER
reader of `context_json`: `course_context_service` harvests
`effective_explanations`, a key no agent has ever written, so that column has
persisted an empty array for every offering since it existed. Out of scope
here, filed as #572.
Hermetic 2184 passed / 9 skipped, ruff clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`test_save_quiz_context_upserts_one_row_and_encrypts` asserts the payload
round-trips exactly, so server-stamping `schema_version` in
`save_quiz_context` changed what comes back. The hermetic twin was updated
with the fix; this is its real-DB counterpart, and only the integration lane
could see it — which is the whole argument for that lane.
Integration 56 passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@AndresL230

Copy link
Copy Markdown
CollaboratorAuthor

Review round — three findings, all fixed

All three were about the schema-version half, not the profile.

1. The version was the model's to change.schema_version was a field on the digest agent's output_type, and submit_quiz persists model_dump() verbatim — so the model owned it. Worse, this PR's own change feeds the previous digest back into the prompt under "update your notes", so from here on the model would seeschema_version: 2 and be invited to update it. A model that helpfully bumped it to 3 would trip the reader's unknown-shape warning on every later read for that (user, concept), forever, with no real drift; one that lowered it would kill the guard just as quietly. The field is off the agent schema entirely now (it also stops costing decoding budget), and save_quiz_context stamps it server-side — which covers every future writer rather than just this one.

2. The guard couldn't catch the drift it cited.version > DIGEST_SCHEMA_VERSION only fires for a writer newer than the reader — a mixed-deploy window. But #548 was a key rename at the same version, and catching that with a version number requires remembering to bump it in the same commit as the rename, which is exactly the discipline that failed the first time. The version now claims only what it can prove, and the rename case is caught by its outcome: a row that is present, stamped with a version this reader understands, and yet yields nothing readable. A real digest can't be all three at once — the agent always writes at least questions_seen_summary or notes — so that combination means the keys moved. Logged with the actual key set, so the next person sees what it drifted to.

3. The docstring understated the blast radius. It said this runs in the post-submit BackgroundTask. It doesn't: submit_quiz builds the whole prompt string synchronously and hands only the finished string to add_task. So an escaping exception 500s the submit after the atomic completed_at claim, the apply_graph_update mastery write and the score update have all landed — the student sees a failed submit for a quiz that scored, and their retry gets a 409 with the score never returned. The guard was right; the comment beside it invited a future reader to delete it as cheap background-task failure.

A live instance of the same bug class, elsewhere

The review also found that course_context_service._parse_quiz_context_to_arrays harvests effective_explanations from context_jsona key no agent has ever written. Its two siblings in the same function read the correct live keys, which is what makes it easy to miss. So offering_concept_stats.effective_explanations has persisted an empty array for every offering since the column existed, indistinguishable from "this class produced none".

I verified it before filing (common_misconceptionscommon_mistakes ✅, prerequisite_gapsweak_areas ✅, effective_explanations ← dead ❌). Out of scope here, filed as #572 — it needs a product decision (make it real vs. delete it) rather than a quiet patch.

The integration lane earned its keep

Server-stamping the version broke test_save_quiz_context_upserts_one_row_and_encrypts, which round-trips the payload against a real database. The hermetic twin was updated alongside the fix; only the real-DB lane caught the second one.

Verification

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

@AndresL230
AndresL230 merged commit e139954 into mainAug 22, 2026
8 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

quiz H2: mine answers_json into the digest (distractor profile) + digest schema version

1 participant

@AndresL230
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' feat(quiz): mine answers_json into the digest as a mistake profile (#554) by AndresL230 · Pull Request #571 · SaplingLearn/Sapling · GitHub
Skip to content

feat(quiz): mine answers_json into the digest as a mistake profile (#554) - #571

Merged
AndresL230 merged 3 commits into
mainfrom
feat/554-distractor-profile-digest
Aug 22, 2026
Merged

feat(quiz): mine answers_json into the digest as a mistake profile (#554)#571
AndresL230 merged 3 commits into
mainfrom
feat/554-distractor-profile-digest

Conversation

@AndresL230

Copy link
Copy Markdown
Collaborator

Closes#554. Workstream H2 of epic #537.

The gold that was never read

quiz_attempts.answers_json has recorded which distractor a student picked, per question, on every submit since the column existed. Nothing has ever read it back.

What the post-submit digest agent actually received was resultslabels only:

{ "question_id": "3", "selected": "B", "correct": false, "correct_answer": "C" }

"Picked B, the answer was C" is not something a model can turn into a misconception. It can only guess one. The option text — the thing that makes a wrong answer mean something — was one join away in questions_json the whole time.

The fix

services/quiz_distractors.py does that join and hands the digest the wrong answers in words: the stem, the concept, what the student chose, and what was correct.

Deliberately dumb — no model call, no I/O, pure data — and it never raises, because it runs inside the post-submit BackgroundTask after the attempt is already graded and written. A crash there would cost the student the digest for a quiz they had already finished.

Per the issue: the join happens in the post-submit task, not at generation, so generation pays nothing for it.

Four things it deliberately does not report

Each would teach the digest something false:

casewhy it's excluded
correct answersa "mistake profile" made of right answers spends tokens reporting the absence of a problem
unanswered questionsskipped is not wrong — recording a blank as a distractor choice invents a misconception out of silence
items with no correct option (#129's shape, which grade wrong for everyone)"the correct answer was nothing" is not a fact about the student
results whose question isn't in the attemptnothing to say

Digest schema version

The other half of the issue. DIGEST_SCHEMA_VERSION is stamped into context_jsonby the model's default rather than by the LLM — a field the model has to remember to set is a field that goes missing — and the reader warns when it meets a version it doesn't understand.

The drift this exists to catch is #548's: the coercer looked for common_errors while the agent wrote common_mistakes, and the symptom was an empty digest, which is indistinguishable from a new student. Unversioned rows are pre-#554 and expected, not a discrepancy.

Tests

Ten, and they pin the wiring as well as the logic — because str.replace on a renamed placeholder is a silent no-op, so the profile would be computed, serialized, and dropped on the floor with nothing to show for it. Also covers the garbage-in cases, since this code's contract is that it cannot throw.

Verification

Hermetic 2181 passed / 9 skipped, ruff clean, oracles 0 findings, integration 56 passed, Playwright 46 passed.

Two Playwright failures, neither from this PR:

🤖 Generated with Claude Code

)
`quiz_attempts.answers_json` has recorded which distractor a student picked,
per question, on every submit since the column existed — and nothing has ever
read it back.
What the post-submit digest agent actually received was `results`, which
carries LABELS: "question 3, picked B, the answer was C". That is not
something a model can turn into a misconception; it can only guess one. The
option TEXT — the thing that makes a wrong answer mean something — was one
join away in `questions_json` the whole time.
`services/quiz_distractors.py` does that join and hands the digest the wrong
answers in words: the stem, the concept, what the student chose, and what was
correct. Deliberately dumb — no model call, no I/O, pure data — and it never
raises, because it runs in the post-submit BackgroundTask after the attempt
is already graded and written, where a crash would cost the student the
digest for a quiz they had already finished.
Four cases it deliberately does NOT report, each of which would teach the
digest something false:
* correct answers — a mistake profile made of right answers spends tokens
to report the absence of a problem;
* unanswered questions — skipped is not wrong, and recording a blank as a
distractor choice invents a misconception out of silence;
* items with no correct option (#129's shape, which grade as wrong for
everyone) — "the correct answer was <nothing>" is not a fact about the
student;
* results whose question is missing from the attempt.
Digest schema version, the other half of the issue: `DIGEST_SCHEMA_VERSION`
is stamped into `context_json` by the model's default rather than by the LLM
(a field the model must remember to set is a field that goes missing), and
the reader warns when it meets a version it doesn't understand. The drift
this exists to catch is #548's: the coercer looked for `common_errors` while
the agent wrote `common_mistakes`, and the symptom was an empty digest —
indistinguishable from a new student.
Tests pin the wiring as well as the logic, because `str.replace` on a
renamed placeholder is a silent no-op: the profile would be computed,
serialized, and dropped on the floor with nothing to show for it.
Hermetic 2181 passed / 9 skipped, ruff clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Aug 22, 2026

Copy link
Copy Markdown

Warning

Review limit reached

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

Next review available in:26 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: d01c32fe-14bd-4aba-9137-f8e87d141887

📥 Commits

Reviewing files that changed from the base of the PR and between c7f043c and bdbcf4f.

📒 Files selected for processing (8)
  • backend/agents/tools/quiz_history.py
  • backend/prompts/quiz_context_update.txt
  • backend/routes/quiz.py
  • backend/services/quiz_context_service.py
  • backend/services/quiz_distractors.py
  • backend/tests/integration/test_quiz_context_repair_db.py
  • backend/tests/test_quiz_context_service.py
  • backend/tests/test_quiz_distractor_profile.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

supabaseBot commented Aug 22, 2026

Copy link
Copy Markdown

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 ↗︎.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Aug 22, 2026

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

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

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend-stagingbdbcf4fCommit Preview URL

Branch Preview URL
Aug 22 2026, 09:03 AM

AndresL230and others added 2 commits August 22, 2026 04:54
Three findings, all about the schema-version half rather than the profile.
**The version was LLM-controlled.** `schema_version` was a field on the
digest agent's output_type, and `submit_quiz` persists `model_dump()`
verbatim. Worse, this PR's own prompt feeds the previous digest back in under
"update your notes" — so from now on the model would SEE `schema_version: 2`
and be invited to update it. A model that helpfully bumped it to 3 would trip
the reader's unknown-shape warning on every later read for that
(user, concept), forever, with no real drift; one that lowered it would kill
the guard just as quietly. The field is now off the agent schema entirely
(so it costs no decoding budget either) and `save_quiz_context` stamps it
server-side, which also covers every future writer.
**The guard couldn't catch the drift it cited.** A version comparison only
fires for a writer NEWER than the reader — a mixed-deploy window. #548 was a
key RENAME at the same version, and catching that with a version number
requires remembering to bump it in the same commit as the rename, which is
precisely the discipline that failed the first time. The version now claims
only what it can prove, and the rename case is caught by its OUTCOME: a row
that is present, stamped with a version this reader understands, and yet
yields nothing readable. A real digest cannot be all three at once — the
agent always writes at least `questions_seen_summary` or `notes` — so that
combination means the keys moved. Logged with the actual key set.
**The docstring understated the blast radius.** It claimed this runs in the
post-submit BackgroundTask. It does not: `submit_quiz` builds the whole
prompt string synchronously and hands only the finished string to
`add_task`. So an escaping exception 500s the submit AFTER the atomic
`completed_at` claim, the mastery write and the score update have landed —
the student sees a failed submit for a quiz that scored, and the retry 409s
with the score never returned. The guard was right; the comment beside it
invited a future reader to delete it as cheap background-task failure.
The review also found a live instance of this same drift class in the OTHER
reader of `context_json`: `course_context_service` harvests
`effective_explanations`, a key no agent has ever written, so that column has
persisted an empty array for every offering since it existed. Out of scope
here, filed as #572.
Hermetic 2184 passed / 9 skipped, ruff clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`test_save_quiz_context_upserts_one_row_and_encrypts` asserts the payload
round-trips exactly, so server-stamping `schema_version` in
`save_quiz_context` changed what comes back. The hermetic twin was updated
with the fix; this is its real-DB counterpart, and only the integration lane
could see it — which is the whole argument for that lane.
Integration 56 passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@AndresL230

Copy link
Copy Markdown
CollaboratorAuthor

Review round — three findings, all fixed

All three were about the schema-version half, not the profile.

1. The version was the model's to change.schema_version was a field on the digest agent's output_type, and submit_quiz persists model_dump() verbatim — so the model owned it. Worse, this PR's own change feeds the previous digest back into the prompt under "update your notes", so from here on the model would seeschema_version: 2 and be invited to update it. A model that helpfully bumped it to 3 would trip the reader's unknown-shape warning on every later read for that (user, concept), forever, with no real drift; one that lowered it would kill the guard just as quietly. The field is off the agent schema entirely now (it also stops costing decoding budget), and save_quiz_context stamps it server-side — which covers every future writer rather than just this one.

2. The guard couldn't catch the drift it cited.version > DIGEST_SCHEMA_VERSION only fires for a writer newer than the reader — a mixed-deploy window. But #548 was a key rename at the same version, and catching that with a version number requires remembering to bump it in the same commit as the rename, which is exactly the discipline that failed the first time. The version now claims only what it can prove, and the rename case is caught by its outcome: a row that is present, stamped with a version this reader understands, and yet yields nothing readable. A real digest can't be all three at once — the agent always writes at least questions_seen_summary or notes — so that combination means the keys moved. Logged with the actual key set, so the next person sees what it drifted to.

3. The docstring understated the blast radius. It said this runs in the post-submit BackgroundTask. It doesn't: submit_quiz builds the whole prompt string synchronously and hands only the finished string to add_task. So an escaping exception 500s the submit after the atomic completed_at claim, the apply_graph_update mastery write and the score update have all landed — the student sees a failed submit for a quiz that scored, and their retry gets a 409 with the score never returned. The guard was right; the comment beside it invited a future reader to delete it as cheap background-task failure.

A live instance of the same bug class, elsewhere

The review also found that course_context_service._parse_quiz_context_to_arrays harvests effective_explanations from context_jsona key no agent has ever written. Its two siblings in the same function read the correct live keys, which is what makes it easy to miss. So offering_concept_stats.effective_explanations has persisted an empty array for every offering since the column existed, indistinguishable from "this class produced none".

I verified it before filing (common_misconceptionscommon_mistakes ✅, prerequisite_gapsweak_areas ✅, effective_explanations ← dead ❌). Out of scope here, filed as #572 — it needs a product decision (make it real vs. delete it) rather than a quiet patch.

The integration lane earned its keep

Server-stamping the version broke test_save_quiz_context_upserts_one_row_and_encrypts, which round-trips the payload against a real database. The hermetic twin was updated alongside the fix; only the real-DB lane caught the second one.

Verification

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

@AndresL230
AndresL230 merged commit e139954 into mainAug 22, 2026
8 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

quiz H2: mine answers_json into the digest (distractor profile) + digest schema version

1 participant

@AndresL230
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' feat(quiz): mine answers_json into the digest as a mistake profile (#554) by AndresL230 · Pull Request #571 · SaplingLearn/Sapling · GitHub
Skip to content

feat(quiz): mine answers_json into the digest as a mistake profile (#554) - #571

Merged
AndresL230 merged 3 commits into
mainfrom
feat/554-distractor-profile-digest
Aug 22, 2026
Merged

feat(quiz): mine answers_json into the digest as a mistake profile (#554)#571
AndresL230 merged 3 commits into
mainfrom
feat/554-distractor-profile-digest

Conversation

@AndresL230

Copy link
Copy Markdown
Collaborator

Closes#554. Workstream H2 of epic #537.

The gold that was never read

quiz_attempts.answers_json has recorded which distractor a student picked, per question, on every submit since the column existed. Nothing has ever read it back.

What the post-submit digest agent actually received was resultslabels only:

{ "question_id": "3", "selected": "B", "correct": false, "correct_answer": "C" }

"Picked B, the answer was C" is not something a model can turn into a misconception. It can only guess one. The option text — the thing that makes a wrong answer mean something — was one join away in questions_json the whole time.

The fix

services/quiz_distractors.py does that join and hands the digest the wrong answers in words: the stem, the concept, what the student chose, and what was correct.

Deliberately dumb — no model call, no I/O, pure data — and it never raises, because it runs inside the post-submit BackgroundTask after the attempt is already graded and written. A crash there would cost the student the digest for a quiz they had already finished.

Per the issue: the join happens in the post-submit task, not at generation, so generation pays nothing for it.

Four things it deliberately does not report

Each would teach the digest something false:

casewhy it's excluded
correct answersa "mistake profile" made of right answers spends tokens reporting the absence of a problem
unanswered questionsskipped is not wrong — recording a blank as a distractor choice invents a misconception out of silence
items with no correct option (#129's shape, which grade wrong for everyone)"the correct answer was nothing" is not a fact about the student
results whose question isn't in the attemptnothing to say

Digest schema version

The other half of the issue. DIGEST_SCHEMA_VERSION is stamped into context_jsonby the model's default rather than by the LLM — a field the model has to remember to set is a field that goes missing — and the reader warns when it meets a version it doesn't understand.

The drift this exists to catch is #548's: the coercer looked for common_errors while the agent wrote common_mistakes, and the symptom was an empty digest, which is indistinguishable from a new student. Unversioned rows are pre-#554 and expected, not a discrepancy.

Tests

Ten, and they pin the wiring as well as the logic — because str.replace on a renamed placeholder is a silent no-op, so the profile would be computed, serialized, and dropped on the floor with nothing to show for it. Also covers the garbage-in cases, since this code's contract is that it cannot throw.

Verification

Hermetic 2181 passed / 9 skipped, ruff clean, oracles 0 findings, integration 56 passed, Playwright 46 passed.

Two Playwright failures, neither from this PR:

🤖 Generated with Claude Code

)
`quiz_attempts.answers_json` has recorded which distractor a student picked,
per question, on every submit since the column existed — and nothing has ever
read it back.
What the post-submit digest agent actually received was `results`, which
carries LABELS: "question 3, picked B, the answer was C". That is not
something a model can turn into a misconception; it can only guess one. The
option TEXT — the thing that makes a wrong answer mean something — was one
join away in `questions_json` the whole time.
`services/quiz_distractors.py` does that join and hands the digest the wrong
answers in words: the stem, the concept, what the student chose, and what was
correct. Deliberately dumb — no model call, no I/O, pure data — and it never
raises, because it runs in the post-submit BackgroundTask after the attempt
is already graded and written, where a crash would cost the student the
digest for a quiz they had already finished.
Four cases it deliberately does NOT report, each of which would teach the
digest something false:
* correct answers — a mistake profile made of right answers spends tokens
to report the absence of a problem;
* unanswered questions — skipped is not wrong, and recording a blank as a
distractor choice invents a misconception out of silence;
* items with no correct option (#129's shape, which grade as wrong for
everyone) — "the correct answer was <nothing>" is not a fact about the
student;
* results whose question is missing from the attempt.
Digest schema version, the other half of the issue: `DIGEST_SCHEMA_VERSION`
is stamped into `context_json` by the model's default rather than by the LLM
(a field the model must remember to set is a field that goes missing), and
the reader warns when it meets a version it doesn't understand. The drift
this exists to catch is #548's: the coercer looked for `common_errors` while
the agent wrote `common_mistakes`, and the symptom was an empty digest —
indistinguishable from a new student.
Tests pin the wiring as well as the logic, because `str.replace` on a
renamed placeholder is a silent no-op: the profile would be computed,
serialized, and dropped on the floor with nothing to show for it.
Hermetic 2181 passed / 9 skipped, ruff clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Aug 22, 2026

Copy link
Copy Markdown

Warning

Review limit reached

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

Next review available in:26 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: d01c32fe-14bd-4aba-9137-f8e87d141887

📥 Commits

Reviewing files that changed from the base of the PR and between c7f043c and bdbcf4f.

📒 Files selected for processing (8)
  • backend/agents/tools/quiz_history.py
  • backend/prompts/quiz_context_update.txt
  • backend/routes/quiz.py
  • backend/services/quiz_context_service.py
  • backend/services/quiz_distractors.py
  • backend/tests/integration/test_quiz_context_repair_db.py
  • backend/tests/test_quiz_context_service.py
  • backend/tests/test_quiz_distractor_profile.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

supabaseBot commented Aug 22, 2026

Copy link
Copy Markdown

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 ↗︎.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Aug 22, 2026

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

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

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend-stagingbdbcf4fCommit Preview URL

Branch Preview URL
Aug 22 2026, 09:03 AM

AndresL230and others added 2 commits August 22, 2026 04:54
Three findings, all about the schema-version half rather than the profile.
**The version was LLM-controlled.** `schema_version` was a field on the
digest agent's output_type, and `submit_quiz` persists `model_dump()`
verbatim. Worse, this PR's own prompt feeds the previous digest back in under
"update your notes" — so from now on the model would SEE `schema_version: 2`
and be invited to update it. A model that helpfully bumped it to 3 would trip
the reader's unknown-shape warning on every later read for that
(user, concept), forever, with no real drift; one that lowered it would kill
the guard just as quietly. The field is now off the agent schema entirely
(so it costs no decoding budget either) and `save_quiz_context` stamps it
server-side, which also covers every future writer.
**The guard couldn't catch the drift it cited.** A version comparison only
fires for a writer NEWER than the reader — a mixed-deploy window. #548 was a
key RENAME at the same version, and catching that with a version number
requires remembering to bump it in the same commit as the rename, which is
precisely the discipline that failed the first time. The version now claims
only what it can prove, and the rename case is caught by its OUTCOME: a row
that is present, stamped with a version this reader understands, and yet
yields nothing readable. A real digest cannot be all three at once — the
agent always writes at least `questions_seen_summary` or `notes` — so that
combination means the keys moved. Logged with the actual key set.
**The docstring understated the blast radius.** It claimed this runs in the
post-submit BackgroundTask. It does not: `submit_quiz` builds the whole
prompt string synchronously and hands only the finished string to
`add_task`. So an escaping exception 500s the submit AFTER the atomic
`completed_at` claim, the mastery write and the score update have landed —
the student sees a failed submit for a quiz that scored, and the retry 409s
with the score never returned. The guard was right; the comment beside it
invited a future reader to delete it as cheap background-task failure.
The review also found a live instance of this same drift class in the OTHER
reader of `context_json`: `course_context_service` harvests
`effective_explanations`, a key no agent has ever written, so that column has
persisted an empty array for every offering since it existed. Out of scope
here, filed as #572.
Hermetic 2184 passed / 9 skipped, ruff clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`test_save_quiz_context_upserts_one_row_and_encrypts` asserts the payload
round-trips exactly, so server-stamping `schema_version` in
`save_quiz_context` changed what comes back. The hermetic twin was updated
with the fix; this is its real-DB counterpart, and only the integration lane
could see it — which is the whole argument for that lane.
Integration 56 passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@AndresL230

Copy link
Copy Markdown
CollaboratorAuthor

Review round — three findings, all fixed

All three were about the schema-version half, not the profile.

1. The version was the model's to change.schema_version was a field on the digest agent's output_type, and submit_quiz persists model_dump() verbatim — so the model owned it. Worse, this PR's own change feeds the previous digest back into the prompt under "update your notes", so from here on the model would seeschema_version: 2 and be invited to update it. A model that helpfully bumped it to 3 would trip the reader's unknown-shape warning on every later read for that (user, concept), forever, with no real drift; one that lowered it would kill the guard just as quietly. The field is off the agent schema entirely now (it also stops costing decoding budget), and save_quiz_context stamps it server-side — which covers every future writer rather than just this one.

2. The guard couldn't catch the drift it cited.version > DIGEST_SCHEMA_VERSION only fires for a writer newer than the reader — a mixed-deploy window. But #548 was a key rename at the same version, and catching that with a version number requires remembering to bump it in the same commit as the rename, which is exactly the discipline that failed the first time. The version now claims only what it can prove, and the rename case is caught by its outcome: a row that is present, stamped with a version this reader understands, and yet yields nothing readable. A real digest can't be all three at once — the agent always writes at least questions_seen_summary or notes — so that combination means the keys moved. Logged with the actual key set, so the next person sees what it drifted to.

3. The docstring understated the blast radius. It said this runs in the post-submit BackgroundTask. It doesn't: submit_quiz builds the whole prompt string synchronously and hands only the finished string to add_task. So an escaping exception 500s the submit after the atomic completed_at claim, the apply_graph_update mastery write and the score update have all landed — the student sees a failed submit for a quiz that scored, and their retry gets a 409 with the score never returned. The guard was right; the comment beside it invited a future reader to delete it as cheap background-task failure.

A live instance of the same bug class, elsewhere

The review also found that course_context_service._parse_quiz_context_to_arrays harvests effective_explanations from context_jsona key no agent has ever written. Its two siblings in the same function read the correct live keys, which is what makes it easy to miss. So offering_concept_stats.effective_explanations has persisted an empty array for every offering since the column existed, indistinguishable from "this class produced none".

I verified it before filing (common_misconceptionscommon_mistakes ✅, prerequisite_gapsweak_areas ✅, effective_explanations ← dead ❌). Out of scope here, filed as #572 — it needs a product decision (make it real vs. delete it) rather than a quiet patch.

The integration lane earned its keep

Server-stamping the version broke test_save_quiz_context_upserts_one_row_and_encrypts, which round-trips the payload against a real database. The hermetic twin was updated alongside the fix; only the real-DB lane caught the second one.

Verification

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

@AndresL230
AndresL230 merged commit e139954 into mainAug 22, 2026
8 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

quiz H2: mine answers_json into the digest (distractor profile) + digest schema version

1 participant

@AndresL230
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' feat(quiz): mine answers_json into the digest as a mistake profile (#554) by AndresL230 · Pull Request #571 · SaplingLearn/Sapling · GitHub
Skip to content

feat(quiz): mine answers_json into the digest as a mistake profile (#554) - #571

Merged
AndresL230 merged 3 commits into
mainfrom
feat/554-distractor-profile-digest
Aug 22, 2026
Merged

feat(quiz): mine answers_json into the digest as a mistake profile (#554)#571
AndresL230 merged 3 commits into
mainfrom
feat/554-distractor-profile-digest

Conversation

@AndresL230

Copy link
Copy Markdown
Collaborator

Closes#554. Workstream H2 of epic #537.

The gold that was never read

quiz_attempts.answers_json has recorded which distractor a student picked, per question, on every submit since the column existed. Nothing has ever read it back.

What the post-submit digest agent actually received was resultslabels only:

{ "question_id": "3", "selected": "B", "correct": false, "correct_answer": "C" }

"Picked B, the answer was C" is not something a model can turn into a misconception. It can only guess one. The option text — the thing that makes a wrong answer mean something — was one join away in questions_json the whole time.

The fix

services/quiz_distractors.py does that join and hands the digest the wrong answers in words: the stem, the concept, what the student chose, and what was correct.

Deliberately dumb — no model call, no I/O, pure data — and it never raises, because it runs inside the post-submit BackgroundTask after the attempt is already graded and written. A crash there would cost the student the digest for a quiz they had already finished.

Per the issue: the join happens in the post-submit task, not at generation, so generation pays nothing for it.

Four things it deliberately does not report

Each would teach the digest something false:

casewhy it's excluded
correct answersa "mistake profile" made of right answers spends tokens reporting the absence of a problem
unanswered questionsskipped is not wrong — recording a blank as a distractor choice invents a misconception out of silence
items with no correct option (#129's shape, which grade wrong for everyone)"the correct answer was nothing" is not a fact about the student
results whose question isn't in the attemptnothing to say

Digest schema version

The other half of the issue. DIGEST_SCHEMA_VERSION is stamped into context_jsonby the model's default rather than by the LLM — a field the model has to remember to set is a field that goes missing — and the reader warns when it meets a version it doesn't understand.

The drift this exists to catch is #548's: the coercer looked for common_errors while the agent wrote common_mistakes, and the symptom was an empty digest, which is indistinguishable from a new student. Unversioned rows are pre-#554 and expected, not a discrepancy.

Tests

Ten, and they pin the wiring as well as the logic — because str.replace on a renamed placeholder is a silent no-op, so the profile would be computed, serialized, and dropped on the floor with nothing to show for it. Also covers the garbage-in cases, since this code's contract is that it cannot throw.

Verification

Hermetic 2181 passed / 9 skipped, ruff clean, oracles 0 findings, integration 56 passed, Playwright 46 passed.

Two Playwright failures, neither from this PR:

🤖 Generated with Claude Code

)
`quiz_attempts.answers_json` has recorded which distractor a student picked,
per question, on every submit since the column existed — and nothing has ever
read it back.
What the post-submit digest agent actually received was `results`, which
carries LABELS: "question 3, picked B, the answer was C". That is not
something a model can turn into a misconception; it can only guess one. The
option TEXT — the thing that makes a wrong answer mean something — was one
join away in `questions_json` the whole time.
`services/quiz_distractors.py` does that join and hands the digest the wrong
answers in words: the stem, the concept, what the student chose, and what was
correct. Deliberately dumb — no model call, no I/O, pure data — and it never
raises, because it runs in the post-submit BackgroundTask after the attempt
is already graded and written, where a crash would cost the student the
digest for a quiz they had already finished.
Four cases it deliberately does NOT report, each of which would teach the
digest something false:
* correct answers — a mistake profile made of right answers spends tokens
to report the absence of a problem;
* unanswered questions — skipped is not wrong, and recording a blank as a
distractor choice invents a misconception out of silence;
* items with no correct option (#129's shape, which grade as wrong for
everyone) — "the correct answer was <nothing>" is not a fact about the
student;
* results whose question is missing from the attempt.
Digest schema version, the other half of the issue: `DIGEST_SCHEMA_VERSION`
is stamped into `context_json` by the model's default rather than by the LLM
(a field the model must remember to set is a field that goes missing), and
the reader warns when it meets a version it doesn't understand. The drift
this exists to catch is #548's: the coercer looked for `common_errors` while
the agent wrote `common_mistakes`, and the symptom was an empty digest —
indistinguishable from a new student.
Tests pin the wiring as well as the logic, because `str.replace` on a
renamed placeholder is a silent no-op: the profile would be computed,
serialized, and dropped on the floor with nothing to show for it.
Hermetic 2181 passed / 9 skipped, ruff clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Aug 22, 2026

Copy link
Copy Markdown

Warning

Review limit reached

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

Next review available in:26 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: d01c32fe-14bd-4aba-9137-f8e87d141887

📥 Commits

Reviewing files that changed from the base of the PR and between c7f043c and bdbcf4f.

📒 Files selected for processing (8)
  • backend/agents/tools/quiz_history.py
  • backend/prompts/quiz_context_update.txt
  • backend/routes/quiz.py
  • backend/services/quiz_context_service.py
  • backend/services/quiz_distractors.py
  • backend/tests/integration/test_quiz_context_repair_db.py
  • backend/tests/test_quiz_context_service.py
  • backend/tests/test_quiz_distractor_profile.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

supabaseBot commented Aug 22, 2026

Copy link
Copy Markdown

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 ↗︎.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Aug 22, 2026

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

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

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend-stagingbdbcf4fCommit Preview URL

Branch Preview URL
Aug 22 2026, 09:03 AM

AndresL230and others added 2 commits August 22, 2026 04:54
Three findings, all about the schema-version half rather than the profile.
**The version was LLM-controlled.** `schema_version` was a field on the
digest agent's output_type, and `submit_quiz` persists `model_dump()`
verbatim. Worse, this PR's own prompt feeds the previous digest back in under
"update your notes" — so from now on the model would SEE `schema_version: 2`
and be invited to update it. A model that helpfully bumped it to 3 would trip
the reader's unknown-shape warning on every later read for that
(user, concept), forever, with no real drift; one that lowered it would kill
the guard just as quietly. The field is now off the agent schema entirely
(so it costs no decoding budget either) and `save_quiz_context` stamps it
server-side, which also covers every future writer.
**The guard couldn't catch the drift it cited.** A version comparison only
fires for a writer NEWER than the reader — a mixed-deploy window. #548 was a
key RENAME at the same version, and catching that with a version number
requires remembering to bump it in the same commit as the rename, which is
precisely the discipline that failed the first time. The version now claims
only what it can prove, and the rename case is caught by its OUTCOME: a row
that is present, stamped with a version this reader understands, and yet
yields nothing readable. A real digest cannot be all three at once — the
agent always writes at least `questions_seen_summary` or `notes` — so that
combination means the keys moved. Logged with the actual key set.
**The docstring understated the blast radius.** It claimed this runs in the
post-submit BackgroundTask. It does not: `submit_quiz` builds the whole
prompt string synchronously and hands only the finished string to
`add_task`. So an escaping exception 500s the submit AFTER the atomic
`completed_at` claim, the mastery write and the score update have landed —
the student sees a failed submit for a quiz that scored, and the retry 409s
with the score never returned. The guard was right; the comment beside it
invited a future reader to delete it as cheap background-task failure.
The review also found a live instance of this same drift class in the OTHER
reader of `context_json`: `course_context_service` harvests
`effective_explanations`, a key no agent has ever written, so that column has
persisted an empty array for every offering since it existed. Out of scope
here, filed as #572.
Hermetic 2184 passed / 9 skipped, ruff clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`test_save_quiz_context_upserts_one_row_and_encrypts` asserts the payload
round-trips exactly, so server-stamping `schema_version` in
`save_quiz_context` changed what comes back. The hermetic twin was updated
with the fix; this is its real-DB counterpart, and only the integration lane
could see it — which is the whole argument for that lane.
Integration 56 passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@AndresL230

Copy link
Copy Markdown
CollaboratorAuthor

Review round — three findings, all fixed

All three were about the schema-version half, not the profile.

1. The version was the model's to change.schema_version was a field on the digest agent's output_type, and submit_quiz persists model_dump() verbatim — so the model owned it. Worse, this PR's own change feeds the previous digest back into the prompt under "update your notes", so from here on the model would seeschema_version: 2 and be invited to update it. A model that helpfully bumped it to 3 would trip the reader's unknown-shape warning on every later read for that (user, concept), forever, with no real drift; one that lowered it would kill the guard just as quietly. The field is off the agent schema entirely now (it also stops costing decoding budget), and save_quiz_context stamps it server-side — which covers every future writer rather than just this one.

2. The guard couldn't catch the drift it cited.version > DIGEST_SCHEMA_VERSION only fires for a writer newer than the reader — a mixed-deploy window. But #548 was a key rename at the same version, and catching that with a version number requires remembering to bump it in the same commit as the rename, which is exactly the discipline that failed the first time. The version now claims only what it can prove, and the rename case is caught by its outcome: a row that is present, stamped with a version this reader understands, and yet yields nothing readable. A real digest can't be all three at once — the agent always writes at least questions_seen_summary or notes — so that combination means the keys moved. Logged with the actual key set, so the next person sees what it drifted to.

3. The docstring understated the blast radius. It said this runs in the post-submit BackgroundTask. It doesn't: submit_quiz builds the whole prompt string synchronously and hands only the finished string to add_task. So an escaping exception 500s the submit after the atomic completed_at claim, the apply_graph_update mastery write and the score update have all landed — the student sees a failed submit for a quiz that scored, and their retry gets a 409 with the score never returned. The guard was right; the comment beside it invited a future reader to delete it as cheap background-task failure.

A live instance of the same bug class, elsewhere

The review also found that course_context_service._parse_quiz_context_to_arrays harvests effective_explanations from context_jsona key no agent has ever written. Its two siblings in the same function read the correct live keys, which is what makes it easy to miss. So offering_concept_stats.effective_explanations has persisted an empty array for every offering since the column existed, indistinguishable from "this class produced none".

I verified it before filing (common_misconceptionscommon_mistakes ✅, prerequisite_gapsweak_areas ✅, effective_explanations ← dead ❌). Out of scope here, filed as #572 — it needs a product decision (make it real vs. delete it) rather than a quiet patch.

The integration lane earned its keep

Server-stamping the version broke test_save_quiz_context_upserts_one_row_and_encrypts, which round-trips the payload against a real database. The hermetic twin was updated alongside the fix; only the real-DB lane caught the second one.

Verification

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

@AndresL230
AndresL230 merged commit e139954 into mainAug 22, 2026
8 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

quiz H2: mine answers_json into the digest (distractor profile) + digest schema version

1 participant

@AndresL230
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' feat(quiz): mine answers_json into the digest as a mistake profile (#554) by AndresL230 · Pull Request #571 · SaplingLearn/Sapling · GitHub
Skip to content

feat(quiz): mine answers_json into the digest as a mistake profile (#554) - #571

Merged
AndresL230 merged 3 commits into
mainfrom
feat/554-distractor-profile-digest
Aug 22, 2026
Merged

feat(quiz): mine answers_json into the digest as a mistake profile (#554)#571
AndresL230 merged 3 commits into
mainfrom
feat/554-distractor-profile-digest

Conversation

@AndresL230

Copy link
Copy Markdown
Collaborator

Closes#554. Workstream H2 of epic #537.

The gold that was never read

quiz_attempts.answers_json has recorded which distractor a student picked, per question, on every submit since the column existed. Nothing has ever read it back.

What the post-submit digest agent actually received was resultslabels only:

{ "question_id": "3", "selected": "B", "correct": false, "correct_answer": "C" }

"Picked B, the answer was C" is not something a model can turn into a misconception. It can only guess one. The option text — the thing that makes a wrong answer mean something — was one join away in questions_json the whole time.

The fix

services/quiz_distractors.py does that join and hands the digest the wrong answers in words: the stem, the concept, what the student chose, and what was correct.

Deliberately dumb — no model call, no I/O, pure data — and it never raises, because it runs inside the post-submit BackgroundTask after the attempt is already graded and written. A crash there would cost the student the digest for a quiz they had already finished.

Per the issue: the join happens in the post-submit task, not at generation, so generation pays nothing for it.

Four things it deliberately does not report

Each would teach the digest something false:

casewhy it's excluded
correct answersa "mistake profile" made of right answers spends tokens reporting the absence of a problem
unanswered questionsskipped is not wrong — recording a blank as a distractor choice invents a misconception out of silence
items with no correct option (#129's shape, which grade wrong for everyone)"the correct answer was nothing" is not a fact about the student
results whose question isn't in the attemptnothing to say

Digest schema version

The other half of the issue. DIGEST_SCHEMA_VERSION is stamped into context_jsonby the model's default rather than by the LLM — a field the model has to remember to set is a field that goes missing — and the reader warns when it meets a version it doesn't understand.

The drift this exists to catch is #548's: the coercer looked for common_errors while the agent wrote common_mistakes, and the symptom was an empty digest, which is indistinguishable from a new student. Unversioned rows are pre-#554 and expected, not a discrepancy.

Tests

Ten, and they pin the wiring as well as the logic — because str.replace on a renamed placeholder is a silent no-op, so the profile would be computed, serialized, and dropped on the floor with nothing to show for it. Also covers the garbage-in cases, since this code's contract is that it cannot throw.

Verification

Hermetic 2181 passed / 9 skipped, ruff clean, oracles 0 findings, integration 56 passed, Playwright 46 passed.

Two Playwright failures, neither from this PR:

🤖 Generated with Claude Code

)
`quiz_attempts.answers_json` has recorded which distractor a student picked,
per question, on every submit since the column existed — and nothing has ever
read it back.
What the post-submit digest agent actually received was `results`, which
carries LABELS: "question 3, picked B, the answer was C". That is not
something a model can turn into a misconception; it can only guess one. The
option TEXT — the thing that makes a wrong answer mean something — was one
join away in `questions_json` the whole time.
`services/quiz_distractors.py` does that join and hands the digest the wrong
answers in words: the stem, the concept, what the student chose, and what was
correct. Deliberately dumb — no model call, no I/O, pure data — and it never
raises, because it runs in the post-submit BackgroundTask after the attempt
is already graded and written, where a crash would cost the student the
digest for a quiz they had already finished.
Four cases it deliberately does NOT report, each of which would teach the
digest something false:
* correct answers — a mistake profile made of right answers spends tokens
to report the absence of a problem;
* unanswered questions — skipped is not wrong, and recording a blank as a
distractor choice invents a misconception out of silence;
* items with no correct option (#129's shape, which grade as wrong for
everyone) — "the correct answer was <nothing>" is not a fact about the
student;
* results whose question is missing from the attempt.
Digest schema version, the other half of the issue: `DIGEST_SCHEMA_VERSION`
is stamped into `context_json` by the model's default rather than by the LLM
(a field the model must remember to set is a field that goes missing), and
the reader warns when it meets a version it doesn't understand. The drift
this exists to catch is #548's: the coercer looked for `common_errors` while
the agent wrote `common_mistakes`, and the symptom was an empty digest —
indistinguishable from a new student.
Tests pin the wiring as well as the logic, because `str.replace` on a
renamed placeholder is a silent no-op: the profile would be computed,
serialized, and dropped on the floor with nothing to show for it.
Hermetic 2181 passed / 9 skipped, ruff clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Aug 22, 2026

Copy link
Copy Markdown

Warning

Review limit reached

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

Next review available in:26 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: d01c32fe-14bd-4aba-9137-f8e87d141887

📥 Commits

Reviewing files that changed from the base of the PR and between c7f043c and bdbcf4f.

📒 Files selected for processing (8)
  • backend/agents/tools/quiz_history.py
  • backend/prompts/quiz_context_update.txt
  • backend/routes/quiz.py
  • backend/services/quiz_context_service.py
  • backend/services/quiz_distractors.py
  • backend/tests/integration/test_quiz_context_repair_db.py
  • backend/tests/test_quiz_context_service.py
  • backend/tests/test_quiz_distractor_profile.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

supabaseBot commented Aug 22, 2026

Copy link
Copy Markdown

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 ↗︎.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Aug 22, 2026

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

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

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend-stagingbdbcf4fCommit Preview URL

Branch Preview URL
Aug 22 2026, 09:03 AM

AndresL230and others added 2 commits August 22, 2026 04:54
Three findings, all about the schema-version half rather than the profile.
**The version was LLM-controlled.** `schema_version` was a field on the
digest agent's output_type, and `submit_quiz` persists `model_dump()`
verbatim. Worse, this PR's own prompt feeds the previous digest back in under
"update your notes" — so from now on the model would SEE `schema_version: 2`
and be invited to update it. A model that helpfully bumped it to 3 would trip
the reader's unknown-shape warning on every later read for that
(user, concept), forever, with no real drift; one that lowered it would kill
the guard just as quietly. The field is now off the agent schema entirely
(so it costs no decoding budget either) and `save_quiz_context` stamps it
server-side, which also covers every future writer.
**The guard couldn't catch the drift it cited.** A version comparison only
fires for a writer NEWER than the reader — a mixed-deploy window. #548 was a
key RENAME at the same version, and catching that with a version number
requires remembering to bump it in the same commit as the rename, which is
precisely the discipline that failed the first time. The version now claims
only what it can prove, and the rename case is caught by its OUTCOME: a row
that is present, stamped with a version this reader understands, and yet
yields nothing readable. A real digest cannot be all three at once — the
agent always writes at least `questions_seen_summary` or `notes` — so that
combination means the keys moved. Logged with the actual key set.
**The docstring understated the blast radius.** It claimed this runs in the
post-submit BackgroundTask. It does not: `submit_quiz` builds the whole
prompt string synchronously and hands only the finished string to
`add_task`. So an escaping exception 500s the submit AFTER the atomic
`completed_at` claim, the mastery write and the score update have landed —
the student sees a failed submit for a quiz that scored, and the retry 409s
with the score never returned. The guard was right; the comment beside it
invited a future reader to delete it as cheap background-task failure.
The review also found a live instance of this same drift class in the OTHER
reader of `context_json`: `course_context_service` harvests
`effective_explanations`, a key no agent has ever written, so that column has
persisted an empty array for every offering since it existed. Out of scope
here, filed as #572.
Hermetic 2184 passed / 9 skipped, ruff clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`test_save_quiz_context_upserts_one_row_and_encrypts` asserts the payload
round-trips exactly, so server-stamping `schema_version` in
`save_quiz_context` changed what comes back. The hermetic twin was updated
with the fix; this is its real-DB counterpart, and only the integration lane
could see it — which is the whole argument for that lane.
Integration 56 passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@AndresL230

Copy link
Copy Markdown
CollaboratorAuthor

Review round — three findings, all fixed

All three were about the schema-version half, not the profile.

1. The version was the model's to change.schema_version was a field on the digest agent's output_type, and submit_quiz persists model_dump() verbatim — so the model owned it. Worse, this PR's own change feeds the previous digest back into the prompt under "update your notes", so from here on the model would seeschema_version: 2 and be invited to update it. A model that helpfully bumped it to 3 would trip the reader's unknown-shape warning on every later read for that (user, concept), forever, with no real drift; one that lowered it would kill the guard just as quietly. The field is off the agent schema entirely now (it also stops costing decoding budget), and save_quiz_context stamps it server-side — which covers every future writer rather than just this one.

2. The guard couldn't catch the drift it cited.version > DIGEST_SCHEMA_VERSION only fires for a writer newer than the reader — a mixed-deploy window. But #548 was a key rename at the same version, and catching that with a version number requires remembering to bump it in the same commit as the rename, which is exactly the discipline that failed the first time. The version now claims only what it can prove, and the rename case is caught by its outcome: a row that is present, stamped with a version this reader understands, and yet yields nothing readable. A real digest can't be all three at once — the agent always writes at least questions_seen_summary or notes — so that combination means the keys moved. Logged with the actual key set, so the next person sees what it drifted to.

3. The docstring understated the blast radius. It said this runs in the post-submit BackgroundTask. It doesn't: submit_quiz builds the whole prompt string synchronously and hands only the finished string to add_task. So an escaping exception 500s the submit after the atomic completed_at claim, the apply_graph_update mastery write and the score update have all landed — the student sees a failed submit for a quiz that scored, and their retry gets a 409 with the score never returned. The guard was right; the comment beside it invited a future reader to delete it as cheap background-task failure.

A live instance of the same bug class, elsewhere

The review also found that course_context_service._parse_quiz_context_to_arrays harvests effective_explanations from context_jsona key no agent has ever written. Its two siblings in the same function read the correct live keys, which is what makes it easy to miss. So offering_concept_stats.effective_explanations has persisted an empty array for every offering since the column existed, indistinguishable from "this class produced none".

I verified it before filing (common_misconceptionscommon_mistakes ✅, prerequisite_gapsweak_areas ✅, effective_explanations ← dead ❌). Out of scope here, filed as #572 — it needs a product decision (make it real vs. delete it) rather than a quiet patch.

The integration lane earned its keep

Server-stamping the version broke test_save_quiz_context_upserts_one_row_and_encrypts, which round-trips the payload against a real database. The hermetic twin was updated alongside the fix; only the real-DB lane caught the second one.

Verification

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

@AndresL230
AndresL230 merged commit e139954 into mainAug 22, 2026
8 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

quiz H2: mine answers_json into the digest (distractor profile) + digest schema version

1 participant

@AndresL230
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); feat(quiz): mine answers_json into the digest as a mistake profile (#554) by AndresL230 · Pull Request #571 · SaplingLearn/Sapling · GitHub
Skip to content

feat(quiz): mine answers_json into the digest as a mistake profile (#554) - #571

Merged
AndresL230 merged 3 commits into
mainfrom
feat/554-distractor-profile-digest
Aug 22, 2026
Merged

feat(quiz): mine answers_json into the digest as a mistake profile (#554)#571
AndresL230 merged 3 commits into
mainfrom
feat/554-distractor-profile-digest

Conversation

@AndresL230

Copy link
Copy Markdown
Collaborator

Closes#554. Workstream H2 of epic #537.

The gold that was never read

quiz_attempts.answers_json has recorded which distractor a student picked, per question, on every submit since the column existed. Nothing has ever read it back.

What the post-submit digest agent actually received was resultslabels only:

{ "question_id": "3", "selected": "B", "correct": false, "correct_answer": "C" }

"Picked B, the answer was C" is not something a model can turn into a misconception. It can only guess one. The option text — the thing that makes a wrong answer mean something — was one join away in questions_json the whole time.

The fix

services/quiz_distractors.py does that join and hands the digest the wrong answers in words: the stem, the concept, what the student chose, and what was correct.

Deliberately dumb — no model call, no I/O, pure data — and it never raises, because it runs inside the post-submit BackgroundTask after the attempt is already graded and written. A crash there would cost the student the digest for a quiz they had already finished.

Per the issue: the join happens in the post-submit task, not at generation, so generation pays nothing for it.

Four things it deliberately does not report

Each would teach the digest something false:

casewhy it's excluded
correct answersa "mistake profile" made of right answers spends tokens reporting the absence of a problem
unanswered questionsskipped is not wrong — recording a blank as a distractor choice invents a misconception out of silence
items with no correct option (#129's shape, which grade wrong for everyone)"the correct answer was nothing" is not a fact about the student
results whose question isn't in the attemptnothing to say

Digest schema version

The other half of the issue. DIGEST_SCHEMA_VERSION is stamped into context_jsonby the model's default rather than by the LLM — a field the model has to remember to set is a field that goes missing — and the reader warns when it meets a version it doesn't understand.

The drift this exists to catch is #548's: the coercer looked for common_errors while the agent wrote common_mistakes, and the symptom was an empty digest, which is indistinguishable from a new student. Unversioned rows are pre-#554 and expected, not a discrepancy.

Tests

Ten, and they pin the wiring as well as the logic — because str.replace on a renamed placeholder is a silent no-op, so the profile would be computed, serialized, and dropped on the floor with nothing to show for it. Also covers the garbage-in cases, since this code's contract is that it cannot throw.

Verification

Hermetic 2181 passed / 9 skipped, ruff clean, oracles 0 findings, integration 56 passed, Playwright 46 passed.

Two Playwright failures, neither from this PR:

🤖 Generated with Claude Code

)
`quiz_attempts.answers_json` has recorded which distractor a student picked,
per question, on every submit since the column existed — and nothing has ever
read it back.
What the post-submit digest agent actually received was `results`, which
carries LABELS: "question 3, picked B, the answer was C". That is not
something a model can turn into a misconception; it can only guess one. The
option TEXT — the thing that makes a wrong answer mean something — was one
join away in `questions_json` the whole time.
`services/quiz_distractors.py` does that join and hands the digest the wrong
answers in words: the stem, the concept, what the student chose, and what was
correct. Deliberately dumb — no model call, no I/O, pure data — and it never
raises, because it runs in the post-submit BackgroundTask after the attempt
is already graded and written, where a crash would cost the student the
digest for a quiz they had already finished.
Four cases it deliberately does NOT report, each of which would teach the
digest something false:
* correct answers — a mistake profile made of right answers spends tokens
to report the absence of a problem;
* unanswered questions — skipped is not wrong, and recording a blank as a
distractor choice invents a misconception out of silence;
* items with no correct option (#129's shape, which grade as wrong for
everyone) — "the correct answer was <nothing>" is not a fact about the
student;
* results whose question is missing from the attempt.
Digest schema version, the other half of the issue: `DIGEST_SCHEMA_VERSION`
is stamped into `context_json` by the model's default rather than by the LLM
(a field the model must remember to set is a field that goes missing), and
the reader warns when it meets a version it doesn't understand. The drift
this exists to catch is #548's: the coercer looked for `common_errors` while
the agent wrote `common_mistakes`, and the symptom was an empty digest —
indistinguishable from a new student.
Tests pin the wiring as well as the logic, because `str.replace` on a
renamed placeholder is a silent no-op: the profile would be computed,
serialized, and dropped on the floor with nothing to show for it.
Hermetic 2181 passed / 9 skipped, ruff clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Aug 22, 2026

Copy link
Copy Markdown

Warning

Review limit reached

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

Next review available in:26 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: d01c32fe-14bd-4aba-9137-f8e87d141887

📥 Commits

Reviewing files that changed from the base of the PR and between c7f043c and bdbcf4f.

📒 Files selected for processing (8)
  • backend/agents/tools/quiz_history.py
  • backend/prompts/quiz_context_update.txt
  • backend/routes/quiz.py
  • backend/services/quiz_context_service.py
  • backend/services/quiz_distractors.py
  • backend/tests/integration/test_quiz_context_repair_db.py
  • backend/tests/test_quiz_context_service.py
  • backend/tests/test_quiz_distractor_profile.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

supabaseBot commented Aug 22, 2026

Copy link
Copy Markdown

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 ↗︎.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Aug 22, 2026

Copy link
Copy Markdown

Deploying with Cloudflare Workers Cloudflare Workers

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

StatusNameLatest CommitPreview URLUpdated (UTC)
✅ Deployment successful!
View logs
frontend-stagingbdbcf4fCommit Preview URL

Branch Preview URL
Aug 22 2026, 09:03 AM

AndresL230and others added 2 commits August 22, 2026 04:54
Three findings, all about the schema-version half rather than the profile.
**The version was LLM-controlled.** `schema_version` was a field on the
digest agent's output_type, and `submit_quiz` persists `model_dump()`
verbatim. Worse, this PR's own prompt feeds the previous digest back in under
"update your notes" — so from now on the model would SEE `schema_version: 2`
and be invited to update it. A model that helpfully bumped it to 3 would trip
the reader's unknown-shape warning on every later read for that
(user, concept), forever, with no real drift; one that lowered it would kill
the guard just as quietly. The field is now off the agent schema entirely
(so it costs no decoding budget either) and `save_quiz_context` stamps it
server-side, which also covers every future writer.
**The guard couldn't catch the drift it cited.** A version comparison only
fires for a writer NEWER than the reader — a mixed-deploy window. #548 was a
key RENAME at the same version, and catching that with a version number
requires remembering to bump it in the same commit as the rename, which is
precisely the discipline that failed the first time. The version now claims
only what it can prove, and the rename case is caught by its OUTCOME: a row
that is present, stamped with a version this reader understands, and yet
yields nothing readable. A real digest cannot be all three at once — the
agent always writes at least `questions_seen_summary` or `notes` — so that
combination means the keys moved. Logged with the actual key set.
**The docstring understated the blast radius.** It claimed this runs in the
post-submit BackgroundTask. It does not: `submit_quiz` builds the whole
prompt string synchronously and hands only the finished string to
`add_task`. So an escaping exception 500s the submit AFTER the atomic
`completed_at` claim, the mastery write and the score update have landed —
the student sees a failed submit for a quiz that scored, and the retry 409s
with the score never returned. The guard was right; the comment beside it
invited a future reader to delete it as cheap background-task failure.
The review also found a live instance of this same drift class in the OTHER
reader of `context_json`: `course_context_service` harvests
`effective_explanations`, a key no agent has ever written, so that column has
persisted an empty array for every offering since it existed. Out of scope
here, filed as #572.
Hermetic 2184 passed / 9 skipped, ruff clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`test_save_quiz_context_upserts_one_row_and_encrypts` asserts the payload
round-trips exactly, so server-stamping `schema_version` in
`save_quiz_context` changed what comes back. The hermetic twin was updated
with the fix; this is its real-DB counterpart, and only the integration lane
could see it — which is the whole argument for that lane.
Integration 56 passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@AndresL230

Copy link
Copy Markdown
CollaboratorAuthor

Review round — three findings, all fixed

All three were about the schema-version half, not the profile.

1. The version was the model's to change.schema_version was a field on the digest agent's output_type, and submit_quiz persists model_dump() verbatim — so the model owned it. Worse, this PR's own change feeds the previous digest back into the prompt under "update your notes", so from here on the model would seeschema_version: 2 and be invited to update it. A model that helpfully bumped it to 3 would trip the reader's unknown-shape warning on every later read for that (user, concept), forever, with no real drift; one that lowered it would kill the guard just as quietly. The field is off the agent schema entirely now (it also stops costing decoding budget), and save_quiz_context stamps it server-side — which covers every future writer rather than just this one.

2. The guard couldn't catch the drift it cited.version > DIGEST_SCHEMA_VERSION only fires for a writer newer than the reader — a mixed-deploy window. But #548 was a key rename at the same version, and catching that with a version number requires remembering to bump it in the same commit as the rename, which is exactly the discipline that failed the first time. The version now claims only what it can prove, and the rename case is caught by its outcome: a row that is present, stamped with a version this reader understands, and yet yields nothing readable. A real digest can't be all three at once — the agent always writes at least questions_seen_summary or notes — so that combination means the keys moved. Logged with the actual key set, so the next person sees what it drifted to.

3. The docstring understated the blast radius. It said this runs in the post-submit BackgroundTask. It doesn't: submit_quiz builds the whole prompt string synchronously and hands only the finished string to add_task. So an escaping exception 500s the submit after the atomic completed_at claim, the apply_graph_update mastery write and the score update have all landed — the student sees a failed submit for a quiz that scored, and their retry gets a 409 with the score never returned. The guard was right; the comment beside it invited a future reader to delete it as cheap background-task failure.

A live instance of the same bug class, elsewhere

The review also found that course_context_service._parse_quiz_context_to_arrays harvests effective_explanations from context_jsona key no agent has ever written. Its two siblings in the same function read the correct live keys, which is what makes it easy to miss. So offering_concept_stats.effective_explanations has persisted an empty array for every offering since the column existed, indistinguishable from "this class produced none".

I verified it before filing (common_misconceptionscommon_mistakes ✅, prerequisite_gapsweak_areas ✅, effective_explanations ← dead ❌). Out of scope here, filed as #572 — it needs a product decision (make it real vs. delete it) rather than a quiet patch.

The integration lane earned its keep

Server-stamping the version broke test_save_quiz_context_upserts_one_row_and_encrypts, which round-trips the payload against a real database. The hermetic twin was updated alongside the fix; only the real-DB lane caught the second one.

Verification

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

@AndresL230
AndresL230 merged commit e139954 into mainAug 22, 2026
8 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

quiz H2: mine answers_json into the digest (distractor profile) + digest schema version

1 participant

@AndresL230