[HnR-Autograder:6] Dashboard columns - #7446
Draft
gmorador-tribu wants to merge 3 commits into
Draft
Conversation
Elimpizza
reviewed
Aug 24, 2026
| # Emitted for every assignment rather than only when true: the | ||
| # dashboard picks the shape of its table off it, and an absent key | ||
| # would be indistinguishable from an older backend. | ||
| checkpoint_enabled=assignment.checkpoint_enabled, |
Contributor
There was a problem hiding this comment.
This same field is added in PR #7445 (checkpoint_enabled on APIAssignment + this constructor + the same four full-dict assertions in assignment_test.py), so one of the two PRs has to drop it or we get a guaranteed conflict and two different comments explaining one field.
I'd suggest PR 7445 keeps it: it needs due_date on the same TypedDict anyway, so the two land together there. Then this PR drops js_config_types.py, views/dashboard/api/assignment.py and the test changes, and picks them up on rebase. Happy to do it the other way round if you'd rather
Elimpizza
changed the base branch from
refactor/students-table-variants
to
main
August 24, 2026 14:00
Elimpizza
changed the base branch from
main
to
refactor/students-table-variants
August 24, 2026 14:01
Elimpizza
force-pushed
the
feat/checkpoint-dashboard-columns
branch
from
August 24, 2026 14:04
910669a to
eb5a57b
Compare
Elimpizza
marked this pull request as draft
August 24, 2026 14:09
Elimpizza
force-pushed
the
feat/checkpoint-dashboard-columns
branch
from
August 28, 2026 14:41
eb5a57b to
aa5259b
Compare
Elimpizza
force-pushed
the
feat/checkpoint-dashboard-columns
branch
from
August 28, 2026 15:18
aa5259b to
3ae153a
Compare
Elimpizza
force-pushed
the
feat/checkpoint-dashboard-columns
branch
from
August 28, 2026 15:20
3ae153a to
e270d18
Compare
A Hide & Reveal assignment splits its activity into phases, and its table repeats the same metrics once per phase. That needs a second header row grouping the columns of each phase, which `DataTable` cannot render: it builds its own single `<thead><tr>`, and its `children` prop renders after the body. So `GroupedActivityTable` assembles the same table out of the primitives `frontend-shared` exports — `Table`/`TableHead`/`TableRow`/`TableCell` plus `useOrderedRows` — and spans a group header over the run of adjacent columns which declare it. It does not have the keyboard navigation, row selection or confirm-on-double-click `DataTable` brings; the dashboard does not use those in this table. `OrderableActivityTable` becomes the facade which picks between the two, so callers keep importing one component and the choice is driven by the data: a column set which declares no group renders exactly as before, through `DataTable`. `navigateOnConfirmRow` is passed only to that path rather than spread into both, since the grouped table has no row confirmation to hook into. The column type gains two optional fields. `group` is the header a column is displayed under; the label identifies the group, so two different groups must not share one. `width` exists because `Table` lays out with `table-fixed`, which divides the width evenly: a nine-column table gives a formatted date less room than it needs, and `FormattedDate` does not wrap. The widths go in a `<colgroup>` rather than on a header cell, because with `table-fixed` the layout is taken from the first row — which here is the group row, whose cells span several columns.
A Hide & Reveal assignment is graded per phase: the checkpoint reveals delimit the phases and the last one closes at the due date. Its table repeats the metrics once per phase, each under the header of its phase, and when the assignment is also auto-graded every phase gets its grade plus a final grade. Only the final grade is synced: the LMS gradebook has one grade per assignment. They are two variants rather than one because `gradesToSync` is read off the module to decide whether the assignment grades at all, not called, so a single module cannot make that decision at runtime. The shape of the data follows the API this is being built against: `phase_metrics` is a list with a 1-based `phase`, `ends_at`, the metrics and an optional per-phase grade. `annotation_metrics` keeps holding the totals, so the variants which know nothing about phases are unaffected. Nothing emits `phase_metrics` yet, which is why the table degrades: a phase no student reports gets no columns, and while none is reported the table is exactly the one the assignment would display without checkpoints. That is also the product rule — a section which is not defined gets no column — and it is what keeps this from showing a grid of empty cells against today's backend. The phases are read off the students because that is where the API reports them. The labels are not reported at all, only positions, so they are derived: the last phase is the due date and the others are checkpoints. That is correct for one checkpoint, which is what the MVP has, and `MIN_PHASES` is what keeps the first phase from being called the due date on the day after a reveal, before anybody has activity in the next one. The comment on it says what would settle this for more than one. Rows generate a field per phase, so the row type widens to an index signature and the module interface becomes generic over it. The variants whose fields are all known keep the checked row type, and every generated key goes through `metricKey` so rows and columns cannot drift apart. Registering the new variants also replaces the way the registry resolves one. Predicates which had to exclude each other's capabilities meant adding a variant edited the variants which came before it, which is the opposite of what this registry is for. Now a variant declares the capabilities it handles, the registry detects them in one place and picks the variant which declares exactly that set. Nothing has to be excluded, the order of the list stops mattering, and two variants can only claim the same assignment by declaring the same set — an invariant a test asserts about the registry rather than about a sample of assignments.
The query which picks the instructors to email had no `ORDER BY`, so Postgres returned the rows in whatever physical order it found them in, which depends on what else had been written to `user` before. That made `test_it_sends_digests_for_instructors` pass or fail by luck: it asserts the calls come out sorted by `h_userid`, which only held when the heap happened to be laid out that way. The unit tests all share one database (`tox.ini` points every worker at `lms_tests`) and run under `--dist loadgroup`, so which tests a worker picks up — and therefore what was written to `user` first — depends on the collected test list. Adding the two dashboard API tests in this branch was enough to reshuffle it and turn the test red, with no change to the code it covers. Ordering in the query rather than in the test also makes the order the digests go out in deterministic in production, which it was not.
Elimpizza
force-pushed
the
feat/checkpoint-dashboard-columns
branch
from
September 1, 2026 13:55
e270d18 to
660bee8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The students table of a Hide & Reveal assignment repeats its metrics once per
phase, each under the header of its phase. When the assignment is also
auto-graded every phase gets its own grade plus a final grade; only the final
grade is synced, because the LMS gradebook has one grade per assignment.
Four commits, in order:
GroupedActivityTable— the table with grouped column headers, assembledout of the
frontend-sharedprimitives becauseDataTablecannot render asecond header row.
OrderableActivityTablebecomes the facade which picksbetween the two, driven by the data: a column set which declares no group
renders exactly as it does today, through
DataTable.Predicates which had to exclude each other's capabilities meant adding a
variant edited the variants which came before it. Now a variant declares the
capabilities it handles and the registry picks the one which declares exactly
that set, so nothing has to be excluded and two variants can only claim the
same assignment by declaring the same set.
checkpoint_enabledin the dashboard API — emitted for every assignmentrather than only when true, unlike
auto_grading_config: the frontend readsit to choose between two table shapes, and an absent key would b
indistinguishable from a backend which does not report it yet.
purpose. That query had no
ORDER BY, and adding the two dashboard API testsin this PR reshuffled the xdist distribution (the unit tests all share one
database), which turned a test that had been passing by luck red
this PR's pipeline fails. Happy to move it to its own PR off
mainif you'drather keep this one focused.
Two things to read before reviewing:
phase_metricscontract ithe PoC in [HnR-Autograder:4] Annotations api #7445 and no backend emits it, so the checkpoint varia
fall through to the "no phase reported" branch and only the tests exercise that
code. That is deliberate.
student reports gets no columns, and while none is reported the table is the
one the assignment would display without checkpoints. That is also the product
rule — a section which is not defined gets no column — and it is
this from rendering a grid of empty cells against the current backend.
Deliberately out of scope: column widths and the layout for mor
checkpoint (two checkpoints means twelve columns, and the headers stop fitting),
and the phase labels for more than one checkpoint, which cannot be derived yet
because the API reports a phase's position and never how many the a