Render hygiene, self-clearing error state, and report-flow hardening - #42
Merged
Merged
Conversation
Fixes Doenet#40, Doenet#41; implements most of Doenet#38 and Doenet#34. Error state (Doenet#40): a source error is derived from the source-analysis useMemo instead of being set into state from inside it, so updating `source` from an invalid to a valid activity on the same mounted component leaves the error screen and renders the activity. Runtime attempt-generation errors (whole-activity and per-item) are captured by the reducer instead of throwing — a throw during React's render-phase reducer run would unmount the viewer via an error boundary — and render as a banner while the activity (whose state the failed action left untouched) stays mounted, so no student work is lost and "New attempt" offers a retry that self-clears the error. React Compiler (Doenet#41): the `set-state-in-render` suppression that made the compiler bail on the whole Viewer component is gone, along with the violations it masked: `loadedStateNum` became a reducer-owned `stateVersion` counter, `itemSequence` is memoized *content-stable* (a score report rebuilds the state tree without changing the sequence; keeping the previous identity keeps everything downstream stable), and the render-body `addItemToRender` scheduling was replaced by deriving the set of items allowed to render from `itemsRendered` plus the pagination/visibility rules. `eslint .` passes with zero errors and zero react-hooks suppressions in the render path. Render hygiene (Doenet#38): the Activity tree components are memoized with stable callbacks; the per-item slices of doenetStates / itemAttemptNumbers / answerResponseCountsByItem are extracted at the Activity→SingleDoc boundary (index lookups via a Map); the ActivityViewer serializes `source` for prop-change detection only when its identity changes, and `flags` defaults to a stable identity. Report-flow hardening (from review): the reducer derives a document's sequence position itself (callers no longer pass itemSequence / doenetStateIdx, which required tracking the current sequence in a ref) and ignores reports from documents no longer in the activity — such a stale report used to throw out of the reducer and unmount every item. The DoenetViewer key now includes the frozen state version: externally loaded state (SPLICE.getState) that doesn't change an item's attempt number previously updated only `initialState`, which the viewer treats as seed-only, so the loaded work was silently ignored. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 12, 2026
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.
Fixes #40. Fixes #41. Implements most of #38 and #34.
Error state (#40)
useMemoinstead of being set into state from inside it — updatingsourcefrom an invalid to a valid activity on the same mounted component clears the error and renders the activity (new cypress spec covers the repro).React Compiler (#41)
The
set-state-in-rendersuppression that made the compiler bail on the wholeViewercomponent is gone, along with all three violations it masked:loadedStateNumbecame a reducer-ownedstateVersioncounter (no more setState-in-effect);itemSequenceis memoized content-stable — every score report rebuilds the state tree without changing the id sequence, and keeping the previous identity keepsitemIdsToRender/checkRender/the memoized item subtrees stable across reports;addItemToRenderscheduling was replaced by deriving the set of items allowed to render fromitemsRenderedplus the pagination/visibility rules (the accumulate-in-state version was redundant state).eslint .passes with zero errors and zero react-hooks suppressions in the render path.Render hygiene (#38)
The Activity tree components are memoized with stable callbacks; per-item slices of
doenetStates/itemAttemptNumbers/answerResponseCountsByItemare extracted at the Activity→SingleDoc boundary (index lookups via aMap), so a score report from one document no longer re-renders every item's subtree.ActivityViewerserializessourcefor prop-change detection only when its identity changes (was: every render), andflagsdefaults to a stable identity.Report-flow hardening (from review)
itemSequence/doenetStateIdx, which required tracking the current sequence in a ref with a stale window) and ignores reports from documents no longer in the activity — such a stale report (e.g. an in-flight save racing a new attempt) used to throw out of the reducer and unmount every item.DoenetViewerkey now includes the frozen state version: externally loaded state (SPLICE.getState) that doesn't change an item's attempt number previously updated onlyinitialState, which the viewer treats as seed-only — the loaded work was silently ignored.Behavior note
The derived render schedule drops an in-flight (not-yet-initialized) prefetch item when the user navigates away before it finishes booting; it re-mounts when eligible again. Previously such items stayed scheduled forever. Items that finished rendering are unaffected.
Tests
43 vitest (new: runtime-error capture + self-clearing; stale-report ignored) and 8 cypress component tests (new: error-recovery repro for #40) pass;
eslint .andvite buildclean. Worth a manual pass in the dev harness (pagination, attempt regeneration, load-state) given the thin automated coverage of the render flow noted in #41.🤖 Generated with Claude Code