Skip to content

fix(widgets): stop rendering missing data as if it were real (#568) - #575

Merged
guillermoscript merged 2 commits into
masterfrom
fix/widget-missing-data-568
Jul 27, 2026
Merged

fix(widgets): stop rendering missing data as if it were real (#568)#575
guillermoscript merged 2 commits into
masterfrom
fix/widget-missing-data-568

Conversation

@guillermoscript

@guillermoscript guillermoscript commented Jul 27, 2026

Copy link
Copy Markdown
Owner

What

Three MCP widget states presented absent data as a value a teacher could act on — a raw user id where a name goes, an em dash where an ungraded exam goes, and a filter blamed for an empty course list that has no filter. Fixed, along with three adjacent defects found while fixing them.

Closes #568

Why

Each symptom is the same mistake: a nullable field rendered through a fallback that produces something plausible-looking instead of saying "we don't have this".

1. A student with no name rendered as their user id. profiles.full_name is nullable and fetchProfileNames() deliberately drops blanks, so student_name: null is a normal value on every roster. The roster fell back to student_id.slice(0, 8) for the name and id.slice(0, 2) for the avatar — u-008 and U-. That identifies nobody and reads as corrupted data. submission-grader had the identical fallback.

The honest fix is a placeholder, not a better-looking id. Resolving the real person would mean their email, which lives in auth.users and needs auth.admin.getUserById() — a service-role call these tools cannot make on the caller's RLS-scoped client. So the copy stays generic and, importantly, looks generic: muted italic, so a reviewer can see at a glance which rows are missing a name.

2. "1 exam" beside "—". The em dash is this widget's glyph for "no data" everywhere else (progress %, last active), so reusing it for an exam awaiting a grade read as took an exam, scored nothing.

3. The empty course list blamed a filter. One branch served two situations. With total === 0 there is no filter to blame, the chips have collapsed to a lone All, and a first-run teacher was given no way forward.

Three adjacent defects, fixed here because leaving them makes the fix incoherent

4. The ungraded-exam state was unreachable from real data. In lms_get_student_progress, exam_count was ex.n, and n was incremented only if (s.score != null). An ungraded submission therefore vanished: the student read as "0 exams", not "1 exam, ungraded". Fixing only the widget would have shipped copy that never renders. aggregateExamSubmissions() now counts every submission and averages only the graded ones.

5. exam-submissions declared student_name: z.string() while its tool sends names.get(...) ?? null. A nameless student rendered as an empty cell.

6. lms_list_courses reported a hardcoded total: 0 on an empty page, discarding the real count. With an offset past the end, a teacher who does have courses would get total: 0 — and, after this change, be shown the "create your first course" screen. Fixing the empty state is what made this latent bug user-visible.

How to QA

No database, RLS, auth or migration surface is touched, so no db:reset is needed — these are widget and MCP-tool changes, verified through the dev-only fixture tools.

cd mcp-server
npm install
MCP_DEMO_WIDGETS=1 npx mcp-use dev --port 3009

The widget HTML embeds MCP_SERVER_URL as the origin it loads its own JS from. If that value does not match the port the dev server actually bound to, the screenshot renders another checkout's code and your changes appear to do nothing. Pin the port as above and set MCP_SERVER_URL=http://localhost:3009 in mcp-server/.env.

  1. Roster — the two name/exam cases (fixture row u-008 has no name; Grace Okoye has one ungraded exam):
    npx mcp-use client screenshot --mcp http://localhost:3009/mcp \
      --tool lms_demo_student_progress_roster variant=default --theme dark --output roster.png
    Expect Unnamed student in muted italic with a · avatar — no u-008, no U- — and Grace Okoye reading Ungraded over "1 exam", not .
  2. Course dashboard — first run:
    npx mcp-use client screenshot --mcp http://localhost:3009/mcp \
      --tool lms_demo_course_dashboard variant=empty --theme dark --output empty.png
    Expect "No courses yet", a line of orientation, a Create my first course button, no mention of a filter, and no lone All chip. Repeat with --theme light.
  3. Course dashboard — the filtered-empty case still works: open lms_demo_course_dashboard variant=default in the inspector and click a status chip; every chip matches at least one course, so the grid should never empty out. The "No courses match this filter" branch is what catches a chip that matches nothing, with a Show all N courses reset.
  4. Regression on the two other widgets that show a student: lms_demo_exam_submissions variant=default and lms_demo_submission_grader — names render exactly as before (every fixture row has one).
  5. cd mcp-server && npm test && npm run build.

Screenshots / GIF

Posted as a comment below — before/after for the roster and the course-dashboard empty state, in both themes and in Spanish, plus two A/B GIFs.

Checklist

  • npm run typecheck and npm run test:unit pass — run as mcp-server's own npm run build (which ends in a full type check) and npm test: 66 tests pass (10 of them new here; the rest arrived with feat(mcp): widget UX polish — i18n, colour semantics, CTAs, dead payload data, card alignment (#570) #578). The root-package scripts do not cover mcp-server/, which is not an npm workspace.
  • npm run build passes — in mcp-server/; 18 widgets built, type check clean.
  • Every new tenant-scoped query filters by tenant_id — no query was added or changed in shape; lms_list_courses keeps its existing .eq("tenant_id", …) and only the reported total changed.
  • Tested with every relevant role — these are teacher/admin-facing widgets, exercised through the dev fixture tools, which is the only way to reach the empty and nameless states deterministically. The roster and dashboard tools are already role-gated in src/tool-policy.ts; that gating is untouched.
  • Loading and error states handled — the isPending branches are unchanged; this PR adds the missing empty and absent-value states.
  • New UI strings added to both messages/en.json and messages/es.jsonequivalent done, different file. MCP widgets render outside the Next.js app and have no access to next-intl; since MCP widget UX polish: i18n, colour semantics, CTAs, unused payload data, card alignment #570 / PR feat(mcp): widget UX polish — i18n, colour semantics, CTAs, dead payload data, card alignment (#570) #578 they carry their own STRINGS tables (resources/shared/i18n.tsx). Every string this PR adds — the placeholder name, "Ungraded", and all four empty states — is defined in both en and es and verified in a Spanish render (below).
  • Migration — none.

Reviewer notes

  • exam_count changes meaning — from "graded submissions" to "submissions". It is read only by this widget and this tool's own summary line; no sort, no at-risk rule, and no other tool consumes it. Worth a look because it changes what a number means, not just how it is formatted.
  • The new tests were not executed against the pre-fix tree, because aggregateExamSubmissions() and student-display.ts did not exist before this branch. What they pin down is the behaviour the old code provably lacked: the old reducer created its accumulator but only ever incremented on a non-null score, so the ungraded-only case came out as exam_count: 0 — the assertion of submitted: 1 could not have passed against it.
  • submission-grader/widget.tsx is also touched by submission-grader: ungraded questions render as 0, overrides are invisible, AI confidence is decoration #567 / PR fix(mcp): submission-grader misreports what is graded — ungraded zeros, invisible overrides, decorative confidence (#567) #574. The change here is one line (the raw-id fallback) and is independent of that PR's ?? 0 work, but the two will want a look if they land close together.
  • The "no courses on this page" branch exists only because of fix 6 above; without a real total it was unreachable, and with the old copy it would have blamed a filter for a pagination overshoot.

Rebased onto master 2026-07-28

PR #578 (#570, widget UX polish) and PR #576 landed first and touch all four of these widgets, so this branch was rebased rather than merged. Three things changed in the process, all visible in the diff:

Re-verified after the rebase: npm test 66 passed, npm run build type check clean, and the roster and dashboard re-shot in en and es (screenshots in the comment below refreshed).

@guillermoscript guillermoscript added bug Something isn't working mcp-server MCP server (mcp-server/) tools, widgets, auth labels Jul 27, 2026
@guillermoscript guillermoscript self-assigned this Jul 27, 2026
guillermoscript added a commit that referenced this pull request Jul 27, 2026
Screenshots and A/B GIFs backing PR #575. Kept on the branch rather than
master so the PR carries its own evidence; safe to drop before merge.

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

guillermoscript commented Jul 27, 2026

Copy link
Copy Markdown
Owner Author

Visual evidence

Captured with the dev-only fixture tools (MCP_DEMO_WIDGETS=1) against the two variants named in the issue's Done when, after the rebase onto #578. "Before" was captured on the untouched tree.

1 + 2. Roster — nameless student and ungraded exam

Row u-008 (no full_name) and Grace Okoye (one submission, not yet graded).

Before After
roster before roster after

roster A/B

u-008Unnamed student in muted italic with a · avatar; Grace Okoye's Ungraded over "1 exam". Every named row is untouched — the progress/exam colours come from #578, not from this PR.

Light theme: before · after

3. Course dashboard — the true-empty state

Before After
dashboard before dashboard after

dashboard A/B

"No courses match this filter" (beside a lone, useless All chip) → a first-run state with a way forward. The chip row and the "0 courses total" line are hidden, since neither says anything true here.

Light theme: after

Spanish

Every string added here is defined in en and es (STRINGS tables, per #578) and rendered via lang=es:

Roster Dashboard
roster es dashboard es

Estudiante sin nombre · Sin calificar · Todavía no tienes cursos · Crear mi primer curso.

Regression check

exam-submissions, whose schema changed to student_name: z.string().nullable() — every fixture row has a name and all render as before:

exam submissions

Media lives in docs/qa/568/ on master. This comment replaces an earlier one whose links pointed at the now-deleted branch.

@guillermoscript
guillermoscript marked this pull request as ready for review July 27, 2026 20:40
guillermoscript and others added 2 commits July 28, 2026 00:38
Three widget states presented absent data as a value a teacher could act
on, plus three adjacent defects found while fixing them.

A student with no `profiles.full_name` rendered as a slice of their user
id — `u-008` in the name column, `U-` in the avatar. That is not a name,
identifies nobody, and reads as corrupted data. `student_name: null` is a
normal value (fetchProfileNames drops blanks), so the fallback now goes
through a shared `resources/shared/student-display.ts`: "Unnamed student"
in a muted italic that reads as a placeholder, and a neutral avatar glyph
instead of letters derived from an id. `submission-grader` had the same
`student_id.slice(0, 8)` fallback; `exam-submissions` declared
`student_name` non-nullable while its tool sends null, so a nameless
student rendered as an empty cell.

The roster showed "1 exam" beside an em dash — the same glyph it uses for
"no data" elsewhere — so an ungraded submission read as an exam scored
nothing. It now reads "Ungraded". That state was also unreachable from
real data: `exam_count` was incremented only `if (score != null)`, which
made an ungraded submission vanish entirely and the student read as "0
exams". The aggregation is extracted as `aggregateExamSubmissions()` and
now counts every submission while averaging only the graded ones.

The course dashboard blamed a filter for every empty result, including
the first-run case where no filter exists, the chips have collapsed to a
lone "All", and the teacher is offered no way forward. It now branches:
no courses at all (first-run copy plus a create-course action, chips
hidden), no courses of a server-filtered status, a chip that matched
nothing, and a page past the end of the list. That last case is newly
reachable because `lms_list_courses` was reporting a hardcoded
`total: 0` on an empty page, discarding the real count — which would have
shown a teacher with courses the "create your first course" screen.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VSGmz5frZqzbpQXJxxuqTs
Screenshots and A/B GIFs backing PR #575. Kept on the branch rather than
master so the PR carries its own evidence; safe to drop before merge.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VSGmz5frZqzbpQXJxxuqTs
@guillermoscript
guillermoscript force-pushed the fix/widget-missing-data-568 branch from 2b62234 to c2d21a3 Compare July 27, 2026 22:40
@guillermoscript
guillermoscript merged commit 8119efe into master Jul 27, 2026
2 checks passed
@guillermoscript
guillermoscript deleted the fix/widget-missing-data-568 branch July 27, 2026 22:43
@guillermoscript

Copy link
Copy Markdown
Owner Author

Merged as 8119efe (squash) into master; branch deleted.

Shipped

  • resources/shared/student-display.ts — one place decides how an unnamed student renders. studentDisplayName / studentInitials / isNamedStudent, used by student-progress-roster, submission-grader and exam-submissions. No widget derives anything shown to a human from a user id any more.
  • The roster's exam column distinguishes "has not sat an exam" () from "sat one, waiting on a grade" (Ungraded, amber).
  • aggregateExamSubmissions() in src/tools/analytics.ts counts every submission and averages only the graded ones, so that second state is reachable from real data at all. exam_count changed meaning: submissions, not graded submissions.
  • course-dashboard has four empty states instead of one — no courses at all (first-run copy plus a create-course action, chip row hidden), no courses of a server-filtered status, a chip that matched nothing (with a reset), and a page past the end of the list.
  • exam-submissions schema corrected to student_name: z.string().nullable(), matching what its tool has always sent.
  • lms_list_courses reports the real count on an empty page instead of a hardcoded 0.

Verification at merge: npm test 66 passed (10 new), npm run build type check clean, CI verify green. Roster and dashboard re-shot in light, dark and Spanish after the rebase onto #578 — evidence in the comment above, now served from docs/qa/568/ on master.

Worth remembering. Widget screenshots are only as trustworthy as the port: the dev HTML embeds MCP_SERVER_URL as the origin it loads its own JS from, and mcp-use dev ignores PORT and auto-picks a free one. In a worktree that silently renders another checkout's code — an "after" shot identical to the "before" one, with nothing in the logs. npx mcp-use dev --port <n> with a matching MCP_SERVER_URL is the fix.

guillermoscript added a commit that referenced this pull request Jul 28, 2026
#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
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

widgets: missing data rendered as real data (raw user ids, ungraded exams as "—", filter-blaming empty state)

1 participant