Skip to content

fix(mcp): submission-grader misreports what is graded — ungraded zeros, invisible overrides, decorative confidence (#567) - #574

Merged
guillermoscript merged 4 commits into
masterfrom
fix/submission-grader-grading-states-567
Jul 28, 2026
Merged

fix(mcp): submission-grader misreports what is graded — ungraded zeros, invisible overrides, decorative confidence (#567)#574
guillermoscript merged 4 commits into
masterfrom
fix/submission-grader-grading-states-567

Conversation

@guillermoscript

Copy link
Copy Markdown
Owner

What

Fixes the three ways the submission-grader MCP widget misreported how much of a submission had actually been graded: ungraded questions rendering as zeros, is_overridden never being shown, and AI confidence being styled as decoration.

Closes #567

Why

A teacher opens this widget to answer one question — how much of this is graded, and where should I look? The per-question list was answering it wrong in three places at once.

Ungraded read as a zero. points_earned: null is the tool's encoding for "nobody has graded this question yet". The widget coerced it with ?? 0, so an untouched question rendered 0/15 pts — pixel-identical to a question the student genuinely bombed. In the demo fixture, Q5 has never been looked at and read as a zero, directly contradicting the 4/5 graded summary a few hundred pixels above it.

Overrides were invisible. is_overridden is declared in the widget's schema, arrives populated from lms_get_submission, and nothing rendered it. A teacher who had already corrected the AI on a question had no way to tell which ones they'd touched — so on a second pass they either re-review everything or mistake the AI's number for their own.

Confidence was decoration. 41% and 82% were the same grey text in the same label. Confidence exists to say where to look, and the 41% question was the least prominent thing on screen.

One thing the issue asked me to check turned out to be fine: the server does not sum null as 0. analytics.ts:382-385 only adds to earned/graded when points_earned != null, so total_points_earned and graded_count already agree. What was misleading is the denominator — total_points_possible spans the whole exam, so 60 / 100 reads as "the student lost 40 points" when 15 of those points have simply never been graded. That is presentation, not arithmetic, so it is fixed in the widget (which already has every question row) rather than by changing the tool or the props schema.

How to QA

No db:reset and no login needed — this runs against the built-in demo fixture, which already contains all three states (Q5 ungraded, Q4 overridden, AI confidences of 0.82 and 0.41).

  1. cd mcp-server && MCP_DEMO_WIDGETS=1 npm run dev
  2. In another shell:
    npx mcp-use client screenshot --mcp http://localhost:3000/mcp \
      --tool lms_demo_submission_grader variant=default --theme dark \
      --width 900 --height 1600 --output grader-dark.png
  3. Repeat with --theme light — widget dark: classes resolve via prefers-color-scheme, not the .dark wrapper class, so both themes are worth a look.
  4. Check each acceptance criterion on the render:
    • Q5 shows —/15 pts in muted type plus a Not graded pill — never 0/15.
    • The grade panel carries an amber 15 pts across 1 question not graded yet line, so 60 / 100 can no longer be read as lost points.
    • Q4 carries a Teacher adjusted pill.
    • Q4's 41% renders as an amber 41% confidence · review this pill with a matching border on the card, while Q3's 82% stays quiet grey text.
  5. Optionally open the inspector at http://localhost:3000/inspector and call lms_demo_submission_grader to poke at it interactively.

Screenshots / GIF

No GIF: this change is purely how the widget renders a payload. Nothing about the interaction (the score input, the feedback box, the Save button) changed, so a recording would show a still frame. Before/after in both themes below.

Dark — before / after

Light — before / after

In the "before" shots, look at the bottom card: 0/15 pts on a question nobody has graded. Q4 has no marker at all despite being teacher-adjusted, and its 41% is the same grey as Q3's 82%.

Notes for the reviewer

  • Widget-only. No tool change, no props-schema change, no migration — everything needed was already in the payload.
  • What I deliberately did not do: the issue suggests ideally showing the AI's original suggestion next to the overridden score. The payload does not carry it (points_earned is already the teacher's number once overridden), so that needs a schema + tool change and belongs in its own issue rather than widening this one.
  • Row order is untouched. The issue floats sorting low-confidence answers first; teachers scan by question number, and re-ordering would trade one confusion for another. The amber border surfaces them without moving them.
  • Colour discipline: amber is reserved for "a human should look at this" (low confidence, ungraded points). "Not graded" itself is neutral zinc so the card doesn't turn into undifferentiated yellow next to the existing amber Awaiting review status pill.
  • Pre-existing, untouched: tsc --noEmit in mcp-server/ reports 4 useCallTool generic errors in flashcards, lesson-viewer, practice-player and study-plan. None are in this file and none are new — flagging them as a separate cleanup.

Checklist

  • npm run typecheck and npm run test:unit pass — clean, 611/611 unit tests. mcp-server Vitest also green (39/39).
  • npm run build passes — not run. This change touches no file in the Next.js app (mcp-server/ is a separate workspace with its own build); npm run mcp:build is the relevant build and the mcp-use dev bundler compiled the widget while the screenshots were taken.
  • Every new tenant-scoped query filters by tenant_id — n/a, no queries added; the widget renders props it is handed.
  • Tested with every relevant role — n/a for the render path; the widget is teacher/admin-only via src/tool-policy.ts and that gate is unchanged.
  • Loading and error states handled — the existing isPending and save-error branches are untouched; the new states (ungraded, overridden, low confidence) each have an explicit render.
  • New UI strings added to both messages/en.json and messages/es.json — n/a: MCP widgets are not wired to next-intl and every existing string in this widget is hardcoded English. The new strings follow that convention.
  • Migration — none.

guillermoscript and others added 2 commits July 27, 2026 22:20
…567)

The per-question review list in the submission-grader widget misreported how
much of a submission had actually been graded — the one thing a teacher opens
it to find out.

- An ungraded question (`points_earned: null`) rendered as `0/15 pts`, making
  "nobody has looked at this yet" indistinguishable from "the student got it
  wrong", and contradicting the `4/5 graded` summary a few hundred pixels
  above. It now renders as `—/15 pts` alongside a "Not graded" pill, and the
  `?? 0` coercion is gone.
- The grade panel now states how many points sit in ungraded questions, so the
  `60 / 100` figure can no longer read as "the student lost 40 points" when 15
  of those points have simply never been graded.
- `is_overridden` arrived in the payload and was never rendered, so a teacher
  could not see which scores they had already corrected. Overridden rows now
  carry a "Teacher adjusted" pill.
- AI confidence was plain grey text appended to a label, so 41% and 82% looked
  identical. Confidence below 60% now renders as an amber "review this" pill
  and tints the question's border, while high confidence stays quiet.

The server side needed no change: `lms_get_submission` already excludes null
scores from `total_points_earned` and `graded_count` (analytics.ts:382-385).
The misleading part was the denominator spanning ungraded questions, which is
a presentation concern and is handled in the widget.

Verified against the `lms_demo_submission_grader` default fixture (ungraded Q5,
overridden Q4, confidences 0.82 and 0.41) in both light and dark themes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017Zt1kZKdcWAcsiJdBgnX3D
guillermoscript and others added 2 commits July 28, 2026 03:02
#578 (widget i18n/colour semantics) and #575 (missing-data display) both
rewrote submission-grader/widget.tsx, so every hunk of this branch
conflicted. Resolved by resetting the file to master and re-applying the
#567 fix on top of the new structure rather than reconciling markers.

What changed in the re-application:

- The three new affordances are now translated. "Not graded",
  "Teacher adjusted", the low-confidence pill and the ungraded-points note
  all have en/es entries in this widget's STRINGS table, matching #578's
  convention; a missing Spanish string is a type error.
- The em dash for an ungraded question is no longer hand-written.
  fmt.number() from shared/i18n.tsx already renders null as an em dash, so
  dropping the `?? 0` is now the entire fix at that call site and the
  dash follows the reader's locale like every other number.
- Points and confidence go through fmt.number/fmt.percent, so es renders
  "confianza: 41 %" rather than a hardcoded English percentage.

master's own shared/severity.ts landed the same rule this issue is about —
"a draft course with no lessons has nothing to complete (neutral); a
student who has completed nothing is at 0% and that is the worst case" —
and #575 applied it to the roster while leaving `points_earned ?? 0` in
this widget untouched. The re-applied fix is that doctrine reaching the
per-question list, and isUngraded() now cites it.

Kept from master unchanged: the inline "(82% confidence)" suffix for a
confident score, so only a guess is promoted to a pill.

Verified after the merge: mcp-server tsc --noEmit clean (the 4 pre-existing
useCallTool errors this branch flagged are gone — master fixed them),
66/66 mcp-server tests, and the demo fixture re-shot in dark, light and es.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017Zt1kZKdcWAcsiJdBgnX3D
master's #578 rewrote this widget with i18n, so the PR's original
screenshots no longer show the code under review. Re-shot from the demo
fixture after the merge, plus a Spanish render — the three new
affordances are translated now, and es is the path the old shots could
not have covered.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017Zt1kZKdcWAcsiJdBgnX3D
@guillermoscript

Copy link
Copy Markdown
Owner Author

Rebased onto master — conflict resolved

master moved under this branch: #578 (widget i18n + colour semantics) and #575 (missing-data display) both rewrote submission-grader/widget.tsx, so every hunk here conflicted.

Resolved by resetting the file to master and re-applying the #567 fix on top of the new structure, rather than reconciling conflict markers — the widget's shape changed enough (string table, useStrings/useFormat, studentDisplayName) that a marker-by-marker merge would have produced something neither branch intended.

What changed in the re-application

  • The three new affordances are translated now. "Not graded", "Teacher adjusted", the low-confidence pill and the ungraded-points note all have en/es entries in this widget's STRINGS table, following feat(mcp): widget UX polish — i18n, colour semantics, CTAs, dead payload data, card alignment (#570) #578's convention. Both branches of the table are the same type, so a missing Spanish string is a compile error rather than an English word leaking into a Spanish render.
  • The em dash is no longer hand-written. fmt.number() in shared/i18n.tsx already renders null as an em dash, so dropping the ?? 0 is the entire fix at that call site — —/15 pts now falls out of the shared formatter and follows the reader's locale like every other number.
  • Points and confidence go through fmt.number/fmt.percent, so Spanish reads confianza: 41 % instead of a hardcoded English percentage.
  • Kept master's inline (82% confidence) suffix for a confident score, so only a guess gets promoted to a pill. That preserves feat(mcp): widget UX polish — i18n, colour semantics, CTAs, dead payload data, card alignment (#570) #578's typographic hierarchy while still making the 41% impossible to miss.

Worth noting for the reviewer

master's own resources/shared/severity.ts (from #578) landed the exact rule this issue is about:

A draft course with no lessons has nothing to complete (neutral); a student enrolled for a month who has completed nothing is at 0% and that is the worst case (red). Callers say which they mean by passing null … rather than coercing it to 0.

#575 then applied that rule to the roster — but left points_earned ?? 0 in this widget untouched. So this PR is that same doctrine reaching the per-question list, not a competing idea; isUngraded() now cites it directly.

One thing this merge retires from the PR description: the 4 pre-existing useCallTool tsc errors I flagged in flashcards/lesson-viewer/practice-player/study-plan are gone — master fixed them. mcp-server now typechecks clean, so no separate cleanup issue is needed.

Re-verified after the merge

Check Result
tsc --noEmit (mcp-server) clean — 0 errors
npx vitest run (mcp-server) 66/66
npm run typecheck (root) clean
npm run test:unit (root) 619/619

The original screenshots in the PR body show pre-#578 code, so here is the widget as it stands after the merge. Spanish is new — the old shots could not have covered it, since the widget had no i18n then.

Dark / light

Spanish (lang=es)15 pts en 1 pregunta sin calificar, Sin calificar, Ajustada por el profesor, confianza: 41 % · revisa esta:

All four acceptance criteria still hold: Q5 reads —/15 pts + Not graded (never 0/15), the grade panel carries the ungraded-points note so 60 / 100 cannot read as lost points, Q4 is marked Teacher adjusted, and its 41% is an amber pill with a matching card border while Q3's 82% stays quiet.

One follow-up I deliberately did not fold in

#575 added mcp-server/tests/missing-data-display.test.ts as the home for exactly this class of bug. The natural next step is to extract isUngraded/LOW_CONFIDENCE into a small pure module and pin them there, so "ungraded is not a zero" is regression-proof rather than resting on the screenshots. That is a new file plus a widget edit, which is beyond resolving this conflict — happy to do it here if you would rather it ship with the fix than as a follow-up.

@guillermoscript
guillermoscript merged commit f4ac391 into master Jul 28, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working mcp-server MCP server (mcp-server/) tools, widgets, auth Sub-task

Projects

None yet

Development

Successfully merging this pull request may close these issues.

submission-grader: ungraded questions render as 0, overrides are invisible, AI confidence is decoration

1 participant