perf(render): stabilize cross-domain action identities via currentStateRef

src/context/AppContext.test.tsx

@@ -1,11 +1,12 @@
-import React, { ReactNode } from 'react';
+import React, { ReactNode, useLayoutEffect, useRef } from 'react';
 import { renderHook, act } from '@testing-library/react';
 import { describe, it, expect, vi, beforeEach } from 'vitest';
 import { renderWithRouter } from '../test-utils/renderWithProviders';
-import { AppProvider, useApp, useRoster, useSettings } from './AppContext';
+import { AppProvider, useApp, useRoster, useSettings, useAuthoring } from './AppContext';
 import * as storage from '../store/storage';
 import { storageSync } from '../services/database';
 import type { Rubric, GradeScale } from '../types';
+import { DEFAULT_FORMAT } from '../types';
 
 // vi.hoisted so this spy can be created before vi.mock('../hooks/useToast', ...) runs
 // (vi.mock factories are hoisted above regular imports/consts) and still be referenced
@@ -688,4 +689,165 @@ describe('AppContext', () => {
         expect(rosterRenders.length).toBeGreaterThan(1); // roster consumer re-rendered
         expect(settingsRenders.length).toBe(1); // settings consumer did NOT
     });
+
+    it('roster consumers do not re-render when authoring data (rubrics) changes', () => {
+        // Cross-domain action stability: createStudentRubric / createGroupStudentRubrics read
+        // rubrics via currentStateRef, so their identity (and thus rosterValue) must not change
+        // when the authoring rubrics collection changes.
+        const rosterRenders: number[] = [];
+        let triggerAddRubric: (() => void) | null = null;
+
+        function RosterProbe() {
+            const { students } = useRoster();
+            rosterRenders.push(students.length);
+            return <div data-testid="roster-probe">{students.length}</div>;
+        }
+
+        function TriggerProbe() {
+            const { addRubric } = useAuthoring();
+            triggerAddRubric = () =>
+                addRubric({
+                    name: 'New Rubric',
+                    subject: '',
+                    description: '',
+                    criteria: [],
+                    gradeScaleId: 'default-scale',
+                    format: DEFAULT_FORMAT,
+                    attachmentIds: [],
+                    totalMaxPoints: 100,
+                    scoringMode: 'weighted-percentage',
+                });
+            return null;
+        }
+
+        renderWithRouter(
+            <>
+                <RosterProbe />
+                <TriggerProbe />
+            </>,
+            { withAppProvider: true }
+        );
+
+        expect(rosterRenders.length).toBe(1);
+
+        act(() => {
+            triggerAddRubric!();
+        });
+
+        expect(rosterRenders.length).toBe(1); // roster consumer did NOT re-render
+    });
+
+    it('settings consumers do not re-render when authoring data (grade scales) changes', () => {
+        // getActiveGradeScale reads gradeScales via currentStateRef, so its identity (and thus
+        // settingsValue) must not change when the authoring grade-scales collection changes.
+        const settingsRenders: string[] = [];
+        let triggerAddGradeScale: (() => void) | null = null;
+
+        function SettingsProbe() {
+            const { settings } = useSettings();
+            settingsRenders.push(settings.language);
+            return <div data-testid="settings-probe">{settings.language}</div>;
+        }
+
+        function TriggerProbe() {
+            const { addGradeScale } = useAuthoring();
+            triggerAddGradeScale = () => addGradeScale({ name: 'New Scale', type: 'points', ranges: [] });
+            return null;
+        }
+
+        renderWithRouter(
+            <>
+                <SettingsProbe />
+                <TriggerProbe />
+            </>,
+            { withAppProvider: true }
+        );
+
+        expect(settingsRenders.length).toBe(1);
+
+        act(() => {
+            triggerAddGradeScale!();
+        });
+
+        expect(settingsRenders.length).toBe(1); // settings consumer did NOT re-render
+    });
+
+    it('actions invoked from a layout effect see the latest rubric state', () => {
+        // Regression for currentStateRef sync timing: the provider must refresh the ref in the
+        // layout phase (parent layout effects fire before descendants'), so an action called from
+        // a descendant's useLayoutEffect — e.g. pre-filling a just-created rubric — reads the
+        // fresh snapshot instead of the previous render's.
+        const entryCounts: number[] = [];
+        const rubricIdRef: { current: string } = { current: '' };
+
+        function LayoutEffectProbe() {
+            const { createStudentRubric, createGroupStudentRubrics } = useRoster();
+            const { addRubric } = useAuthoring();
+            const runRef = useRef(0);
+
+            useLayoutEffect(() => {
+                runRef.current += 1;
+                if (runRef.current === 1) {
+                    const rubric = addRubric({
+                        name: 'Layout Rubric',
+                        subject: '',
+                        description: '',
+                        gradeScaleId: 'default-scale',
+                        format: DEFAULT_FORMAT,
+                        attachmentIds: [],
+                        totalMaxPoints: 30,
+                        scoringMode: 'weighted-percentage',
+                        criteria: [
+                            { id: 'c1', title: 'C1', description: '', weight: 34, levels: [] },
+                            { id: 'c2', title: 'C2', description: '', weight: 33, levels: [] },
+                            { id: 'c3', title: 'C3', description: '', weight: 33, levels: [] },
+                        ],
+                    });
+                    rubricIdRef.current = rubric.id;
+                } else if (runRef.current === 2) {
+                    const single = createStudentRubric(rubricIdRef.current, 'student-1');
+                    entryCounts.push(single.entries.length);
+                    const group = createGroupStudentRubrics(rubricIdRef.current, ['student-1', 'student-2']);
+                    entryCounts.push(group[0].entries.length);
+                }
+            });
+
+            return <div data-testid="layout-rubric-probe" />;
+        }
+
+        renderWithRouter(<LayoutEffectProbe />, { withAppProvider: true });
+        act(() => {}); // flush the re-render scheduled by the first layout effect
+
+        // Each created StudentRubric must mirror the 3 criteria of the rubric added in run 1.
+        expect(entryCounts).toEqual([3, 3]);
+    });
+
+    it('getActiveGradeScale invoked from a layout effect sees the latest grade-scale state', () => {
+        const observedNames: string[] = [];
+
+        function LayoutEffectScaleProbe() {
+            const { getActiveGradeScale, updateSettings } = useSettings();
+            const { addGradeScale } = useAuthoring();
+            const runRef = useRef(0);
+
+            useLayoutEffect(() => {
+                runRef.current += 1;
+                if (runRef.current === 1) {
+                    const scale = addGradeScale({ name: 'Layout Scale', type: 'points', ranges: [] });
+                    updateSettings({ defaultGradeScaleId: scale.id });
+                } else if (runRef.current === 2) {
+                    observedNames.push(getActiveGradeScale().name);
+                }
+            });
+
+            return <div data-testid="layout-scale-probe" />;
+        }
+
+        renderWithRouter(<LayoutEffectScaleProbe />, { withAppProvider: true });
+        act(() => {}); // flush the re-render scheduled by the first layout effect
+
+        // With the fix the ref is refreshed in the layout phase, so the action sees the scale
+        // added in run 1; with a passive-effect sync it would still resolve the old default.
+        expect(observedNames).toEqual(['Layout Scale']);
+    });
 });

src/context/AppContext.tsx

@@ -1419,7 +1419,14 @@ async function flushToLocalStorage(merged: StoreData, changedKeys?: Set<keyof St
 export function AppProvider({ children }: { children: ReactNode }) {
     const [state, dispatch] = useReducer(loggingReducer, null, loadStore);
     const initialStateRef = useRef(state);
+    // Latest-state ref (documented "latest ref" pattern, react.dev/useRef): written during
+    // render so it always holds the state of the current render. Actions below read it instead
+    // of closing over a slice (stable identity), and because render-phase writes happen before
+    // every effect, they observe the fresh snapshot even when invoked from a descendant's
+    // useLayoutEffect — an effect-based sync (layout or passive) would lag behind by a commit.
     const currentStateRef = useRef(state);
+    // eslint-disable-next-line react-hooks/refs -- intentional latest-ref write; idempotent per render
+    currentStateRef.current = state;
     // Backs the delta-sync diff effect further below (compares each render's
     // state to the last one to decide what to push to Supabase).
     const prevStateRef = useRef(state);
@@ -1451,11 +1458,6 @@ export function AppProvider({ children }: { children: ReactNode }) {
         });
     }, [showToast, t]);
 
-    // Keep currentStateRef in sync so the reconnect handler always sees fresh state
-    useEffect(() => {
-        currentStateRef.current = state;
-    }, [state]);
-
     // 'checking' while we detect session; 'show' = show landing; 'hide' = in app
     const [landingState, setLandingState] = useState<'checking' | 'show' | 'hide'>('checking');
     // Ref so the OTP handler ([] deps effect) can read current state without
@@ -1918,6 +1920,11 @@ export function AppProvider({ children }: { children: ReactNode }) {
         []
     );
 
+    // These actions read state via currentStateRef instead of closing over a slice so their
+    // identity stays stable: a closure over e.g. `state.rubrics` would re-identify the action
+    // (and thus re-render every roster consumer) whenever rubrics change, even though rubrics
+    // belong to the authoring domain. Read-only helpers built from fresh state keep each domain
+    // value changing only when the slices it exposes change.
     const createStudentRubric = useCallback((rubricId: string, studentId: string): StudentRubric => {
         const rubric = currentStateRef.current.rubrics.find((r) => r.id === rubricId);
         const entries: ScoreEntry[] = (rubric?.criteria ?? []).map((c) => ({
@@ -1943,41 +1950,38 @@ export function AppProvider({ children }: { children: ReactNode }) {
      * Grading any member through the normal single-student flow then duplicates its scores to the
      * rest of the group via the SAVE_STUDENT_RUBRIC reducer case — no separate group grading UI.
      */
-    const createGroupStudentRubrics = useCallback(
-        (rubricId: string, studentIds: string[]): StudentRubric[] => {
-            const rubric = currentStateRef.current.rubrics.find((r) => r.id === rubricId);
-            const entries: ScoreEntry[] = (rubric?.criteria ?? []).map((c) => ({
-                criterionId: c.id,
-                levelId: null,
-                comment: '',
-                checkedSubItems: [],
-            }));
-            const groupId = nanoid();
-            const srs = studentIds.map((studentId): StudentRubric => {
-                const existing = state.studentRubrics.find(
-                    (sr) => sr.rubricId === rubricId && sr.studentId === studentId && !sr.isPeerReview
-                );
-                return {
-                    ...existing,
-                    id: existing?.id ?? nanoid(),
-                    rubricId,
-                    studentId,
-                    entries: entries.map((e) => ({ ...e })),
-                    overallComment: '',
-                    isPeerReview: false,
-                    groupId,
-                    gradedBy: undefined,
-                    gradedAt: undefined,
-                    submittedAt: undefined,
-                    notHandedIn: undefined,
-                    round: undefined,
-                };
-            });
-            srs.forEach((sr) => dispatch({ type: 'SAVE_STUDENT_RUBRIC', payload: sr }));
-            return srs;
-        },
-        [state.rubrics, state.studentRubrics]
-    );
+    const createGroupStudentRubrics = useCallback((rubricId: string, studentIds: string[]): StudentRubric[] => {
+        const rubric = currentStateRef.current.rubrics.find((r) => r.id === rubricId);
+        const entries: ScoreEntry[] = (rubric?.criteria ?? []).map((c) => ({
+            criterionId: c.id,
+            levelId: null,
+            comment: '',
+            checkedSubItems: [],
+        }));
+        const groupId = nanoid();
+        const srs = studentIds.map((studentId): StudentRubric => {
+            const existing = currentStateRef.current.studentRubrics.find(
+                (sr) => sr.rubricId === rubricId && sr.studentId === studentId && !sr.isPeerReview
+            );
+            return {
+                ...existing,
+                id: existing?.id ?? nanoid(),
+                rubricId,
+                studentId,
+                entries: entries.map((e) => ({ ...e })),
+                overallComment: '',
+                isPeerReview: false,
+                groupId,
+                gradedBy: undefined,
+                gradedAt: undefined,
+                submittedAt: undefined,
+                notHandedIn: undefined,
+                round: undefined,
+            };
+        });
+        srs.forEach((sr) => dispatch({ type: 'SAVE_STUDENT_RUBRIC', payload: sr }));
+        return srs;
+    }, []);
 
     const deleteStudentRubric = useCallback((id: string, scope: 'student' | 'group') => {
         logAuditEvent('grade', 'student_rubric_delete', 'student_rubric', id, { scope });
@@ -2011,6 +2015,9 @@ export function AppProvider({ children }: { children: ReactNode }) {
         []
     );
 
+    // Read via currentStateRef for the same reason as createStudentRubric: gradeScales belongs to
+    // the authoring domain, so closing over it would re-render every settings consumer on any
+    // grade-scale change.
     const getActiveGradeScale = useCallback((): GradeScale => {
         const { gradeScales, settings } = currentStateRef.current;
         return gradeScales.find((gs) => gs.id === settings.defaultGradeScaleId) ?? gradeScales[0];