Separate document numbering from scored item numbering

package.json

@@ -2,7 +2,7 @@
     "name": "@doenet/assignment-viewer",
     "private": false,
     "description": "View assignments from questions written in DoenetML",
-    "version": "0.1.0-alpha-17",
+    "version": "0.1.0-alpha-18",
     "license": "AGPL-3.0-or-later",
     "homepage": "https://github.com/Doenet/assignment-viewer#readme",
     "type": "module",

src/Activity/Activity.tsx

@@ -27,13 +27,13 @@ export type ActivityCommonProps = {
     answerResponseCountsByItem?: Record<string, number>[];
     doenetStates: unknown[];
     stateVersion: number;
-    /** The windowed mounting policy every item's viewer registers with. */
+    /** The windowed mounting policy every document's viewer registers with. */
     mountPolicy: MountPolicy;
     useSharedCoreWorker?: boolean;
     reportScoreAndStateCallback: (args: unknown) => void;
     checkHidden: (state: ActivityState) => boolean;
     /**
-     * Whether an item's viewer should stay booted even while hidden or
+     * Whether a document's viewer should stay booted even while hidden or
      * off-screen (the paginator marks the current page and its neighbors
      * so page flips are instant).
      */
@@ -45,7 +45,8 @@ export type ActivityCommonProps = {
     ) => void;
     hasRenderedCallback: (id: string) => void;
     itemAttemptNumbers: number[];
-    itemIndexById: ReadonlyMap<string, number>;
+    /** Position of each scored item, i.e. of each document that isn't a description. */
+    scoredItemIndexById: ReadonlyMap<string, number>;
     itemWord: string;
 };
 
@@ -55,18 +56,20 @@ export const Activity = memo(function Activity({
 }: ActivityCommonProps & { state: ActivityState }) {
     switch (state.type) {
         case "singleDoc": {
-            // Extract this item's slice of the per-item arrays so the leaf's
-            // props only change when *its* data changes: the arrays get a
-            // fresh identity on every report from any document, which would
-            // otherwise defeat SingleDocActivity's memo for all N items.
+            // Extract this document's slice of the per-item arrays so the
+            // leaf's props only change when *its* data changes: the arrays get
+            // a fresh identity on every report from any document, which would
+            // otherwise defeat SingleDocActivity's memo for all N documents.
             const {
                 doenetStates,
                 itemAttemptNumbers,
                 answerResponseCountsByItem = [],
-                itemIndexById,
+                scoredItemIndexById,
                 ...leafProps
             } = props;
-            const itemIdx = itemIndexById.get(state.id) ?? -1;
+            // A description holds no slot in the per-item arrays, so it gets
+            // an out-of-range index and falls back to the defaults below.
+            const scoredItemIdx = scoredItemIndexById.get(state.id) ?? -1;
             return (
                 <SingleDocActivity
                     {...leafProps}
@@ -76,8 +79,10 @@ export const Activity = memo(function Activity({
                             ? null
                             : (doenetStates[state.doenetStateIdx] ?? null)
                     }
-                    itemAttemptNumber={itemAttemptNumbers[itemIdx]}
-                    answerResponseCounts={answerResponseCountsByItem[itemIdx]}
+                    itemAttemptNumber={itemAttemptNumbers[scoredItemIdx] ?? 1}
+                    answerResponseCounts={
+                        answerResponseCountsByItem[scoredItemIdx]
+                    }
                 />
             );
         }

src/Activity/SingleDocActivity.tsx

@@ -8,10 +8,13 @@ type SingleDocActivityProps = Omit<
     | "doenetStates"
     | "itemAttemptNumbers"
     | "answerResponseCountsByItem"
-    | "itemIndexById"
+    | "scoredItemIndexById"
 > & {
     state: SingleDocState;
-    /** This item's saved Doenet state (its slice of `doenetStates`). */
+    /**
+     * This document's saved Doenet state (its slice of `doenetStates`).
+     * Always null for a description, which has no slot in `doenetStates`.
+     */
     doenetState: unknown;
     itemAttemptNumber: number;
     answerResponseCounts?: Record<string, number>;

src/Activity/activityState.ts

@@ -25,14 +25,16 @@ import {
     SelectSource,
     SelectState,
     SelectStateNoSource,
-    getNumItemsInSelect,
+    getNumDocsInSelect,
+    getNumScoredItemsInSelect,
 } from "./selectState";
 import {
     addSourceToSequenceState,
     calcNumVariantsSequence,
     extractSequenceItemCredit,
     generateNewSequenceAttempt,
-    getNumItemsInSequence,
+    getNumDocsInSequence,
+    getNumScoredItemsInSequence,
     initializeSequenceState,
     isSequenceSource,
     isSequenceState,
@@ -240,8 +242,7 @@ export function initializeActivityAndDoenetState({
         restrictToVariantSlice,
     });
 
-    const numItems = getNumItems(source);
-    const itemAttemptNumbers = Array<number>(numItems).fill(1);
+    const itemAttemptNumbers = Array<number>(getNumScoredItems(source)).fill(1);
     return {
         activityState,
         doenetStates: [],
@@ -514,8 +515,11 @@ export function extractSourceId(compositeId: string): string {
 /**
  * Returns an array of the activity ids of the single document activities,
  * in the order they will appear.
+ *
+ * Includes descriptions, which are rendered like any other document.
+ * Use `getScoredItemSequence` for the sequence of scored items.
  */
-export function getItemSequence(state: ActivityState): string[] {
+export function getDocSequence(state: ActivityState): string[] {
     if (state.type === "singleDoc") {
         return [state.id];
     } else {
@@ -525,7 +529,7 @@ export function getItemSequence(state: ActivityState): string[] {
                 return [];
             } else {
                 const prelimResult = state.allChildren.flatMap((a) =>
-                    getItemSequence(a),
+                    getDocSequence(a),
                 );
                 if (state.type === "sequence") {
                     return prelimResult;
@@ -535,13 +539,30 @@ export function getItemSequence(state: ActivityState): string[] {
             }
         }
         if (state.type === "sequence") {
-            return state.orderedChildren.flatMap((a) => getItemSequence(a));
+            return state.orderedChildren.flatMap((a) => getDocSequence(a));
         } else {
-            return state.selectedChildren.flatMap((a) => getItemSequence(a));
+            return state.selectedChildren.flatMap((a) => getDocSequence(a));
         }
     }
 }
 
+/**
+ * Returns an array of the activity ids of the single document activities that count
+ * as scored items, i.e., `getDocSequence` with the descriptions removed.
+ *
+ * The index of an id in this array is its `shuffledOrder` from
+ * `extractActivityItemCredit` minus one, which is the indexing used by
+ * `doenetStates` and `itemAttemptNumbers`.
+ */
+export function getScoredItemSequence(state: ActivityState): string[] {
+    const allStates = gatherStates(state);
+
+    return getDocSequence(state).filter((id) => {
+        const docState = allStates[id];
+        return docState.type !== "singleDoc" || !docState.source.isDescription;
+    });
+}
+
 /**
  * Assuming that `numActivityVariants` contains the number of variants for all single doc activities,
  * calculate the number of unique variants of the the activity given by `source`.
@@ -594,16 +615,39 @@ export function calcNumVariantsFromState(
  *
  * Throw an error if a select has options with different numbers of documents.
  */
-export function getNumItems(source: ActivitySource): number {
+export function getNumDocs(source: ActivitySource): number {
     switch (source.type) {
         case "singleDoc": {
             return 1;
         }
         case "select": {
-            return getNumItemsInSelect(source);
+            return getNumDocsInSelect(source);
+        }
+        case "sequence": {
+            return getNumDocsInSequence(source);
+        }
+    }
+
+    throw Error("Invalid activity type");
+}
+
+/**
+ * Return the number of documents of this activity that count as scored items,
+ * i.e., all rendered documents except the descriptions.
+ *
+ * Throw an error if a select has options with different numbers of documents,
+ * or if a select contains a description.
+ */
+export function getNumScoredItems(source: ActivitySource): number {
+    switch (source.type) {
+        case "singleDoc": {
+            return source.isDescription ? 0 : 1;
+        }
+        case "select": {
+            return getNumScoredItemsInSelect(source);
         }
         case "sequence": {
-            return getNumItemsInSequence(source);
+            return getNumScoredItemsInSequence(source);
         }
     }
 

src/Activity/activityStateReducer.ts

@@ -12,8 +12,8 @@ import {
     gatherStates,
     generateNewActivityAttempt,
     generateNewSingleDocSubAttempt,
-    getItemSequence,
-    getNumItems,
+    getNumScoredItems,
+    getScoredItemSequence,
     initializeActivityState,
     propagateStateChangeToRoot,
     pruneActivityStateForSave,
@@ -77,7 +77,7 @@ export function activityDoenetStateReducer(
     const activityState = state.activityState;
     switch (action.type) {
         case "initialize": {
-            const numItems = getNumItems(action.source);
+            const numScoredItems = getNumScoredItems(action.source);
             return {
                 activityState: initializeActivityState({
                     source: action.source,
@@ -86,7 +86,7 @@ export function activityDoenetStateReducer(
                     numActivityVariants: action.numActivityVariants,
                 }),
                 doenetStates: [],
-                itemAttemptNumbers: Array<number>(numItems).fill(1),
+                itemAttemptNumbers: Array<number>(numScoredItems).fill(1),
                 stateVersion: state.stateVersion + 1,
                 errMsg: null,
             };
@@ -170,12 +170,17 @@ export function activityDoenetStateReducer(
                 );
             }
 
-            // The document's position in the item sequence is derived from
-            // the reducer's own state (callers used to pass it in, which
-            // required them to track the current sequence).
-            const doenetStateIdx = getItemSequence(activityState).indexOf(
+            // The document's position in the scored item sequence is derived
+            // from the reducer's own state, so callers need not track the
+            // current sequence themselves. An id absent from that sequence is
+            // either stale or a description, and neither has a slot in
+            // `doenetStates`/`itemAttemptNumbers` to regenerate.
+            const doenetStateIdx = getScoredItemSequence(activityState).indexOf(
                 action.docId,
             );
+            if (doenetStateIdx === -1) {
+                return state;
+            }
 
             let newActivityState;
             try {
@@ -243,12 +248,14 @@ export function activityDoenetStateReducer(
             };
         }
         case "updateSingleState": {
-            // A report can arrive from a document that is no longer part of
-            // the activity — e.g. an in-flight save from a just-regenerated
-            // attempt, or after a select re-picked its children. Ignore it:
-            // recording it would corrupt another item's slot, and throwing
-            // would unmount the whole viewer via an error boundary.
-            const doenetStateIdx = getItemSequence(activityState).indexOf(
+            // A report with no slot in the scored item sequence is silently
+            // ignored: recording it would corrupt another item's slot, and
+            // throwing would unmount the whole viewer via an error boundary.
+            // Either the document is no longer part of the activity (an
+            // in-flight save from a just-regenerated attempt, or a select that
+            // re-picked its children), or it is a description, which is
+            // unscored and unpersisted.
+            const doenetStateIdx = getScoredItemSequence(activityState).indexOf(
                 action.docId,
             );
             if (doenetStateIdx === -1) {

src/Activity/selectState.ts

@@ -13,7 +13,8 @@ import {
     extractActivityItemCredit,
     extractSourceId,
     generateNewActivityAttempt,
-    getNumItems,
+    getNumDocs,
+    getNumScoredItems,
     initializeActivityState,
     isActivitySource,
     isActivityState,
@@ -742,16 +743,16 @@ export function calcNumVariantsSelect(
  *
  * Throw an error if this select has options with different numbers of documents.
  */
-export function getNumItemsInSelect(source: SelectSource): number {
+export function getNumDocsInSelect(source: SelectSource): number {
     if (source.items.length === 0) {
         return 0;
     }
 
-    const numDocumentsPerItem = getNumItems(source.items[0]);
+    const numDocumentsPerItem = getNumDocs(source.items[0]);
 
     if (source.items.length > 1) {
         for (const item of source.items.slice(1)) {
-            if (getNumItems(item) !== numDocumentsPerItem) {
+            if (getNumDocs(item) !== numDocumentsPerItem) {
                 throw Error(
                     "The case where a select has options with different numbers of documents is not implemented",
                 );
@@ -761,3 +762,25 @@ export function getNumItemsInSelect(source: SelectSource): number {
 
     return source.numToSelect * numDocumentsPerItem;
 }
+
+/**
+ * Return the number of documents of this select that count as scored items.
+ *
+ * Descriptions inside a select are not supported: `extractSelectItemCredit` scores a
+ * single-document select regardless of `isDescription`, and the select branch of
+ * `propagateStateChangeToRoot` averages credit over all selected children without
+ * filtering out descriptions. Rather than silently miscount, throw.
+ */
+export function getNumScoredItemsInSelect(source: SelectSource): number {
+    if (
+        source.items.some(
+            (item) => getNumScoredItems(item) !== getNumDocs(item),
+        )
+    ) {
+        throw Error(
+            "The case where a select contains a description is not implemented",
+        );
+    }
+
+    return getNumDocsInSelect(source);
+}