From 5a8414b1f87402a342923b07d878ed54d2fd1cb5 Mon Sep 17 00:00:00 2001 From: Duane Nykamp Date: Fri, 1 May 2026 14:14:14 -0500 Subject: [PATCH 1/8] refactor(worker-javascript): extract Phase 1 helpers from Core.js MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Begin breaking up the 13,837-line Core class by lifting seven self- contained, low-coupling helpers into TypeScript modules. The pattern matches the existing composed siblings (Dependencies.js, ParameterStack): each module is constructed with a `core` back-reference, and Core retains a thin delegating wrapper for every method/property that was previously on the class so external callers (CoreWorker, tests, components, and `coreFunctions`-bound references) continue to work unchanged. Modules extracted: - DiagnosticsManager.ts — diagnostics queue + source-location walk - StateVariableNameResolver.ts — pure-function name resolution utilities - VisibilityTracker.ts — visibility state and save/suspend timers - StatePersistence.ts — save to localStorage / database - AutoSubmitManager.ts — debounced answer-submit queue - NavigationHandler.ts — handleNavigatingToComponent, navigateToTarget - ResolverAdapter.ts — adapter to the external Rust name resolver No behavior change. Core.js drops from 13,837 to 12,909 lines. Co-Authored-By: Claude Haiku 4.5 --- .../src/AutoSubmitManager.ts | 60 + .../doenetml-worker-javascript/src/Core.js | 1162 ++--------------- .../src/DiagnosticsManager.ts | 182 +++ .../src/NavigationHandler.ts | 66 + .../src/ResolverAdapter.ts | 459 +++++++ .../src/StatePersistence.ts | 149 +++ .../src/StateVariableNameResolver.ts | 275 ++++ .../src/VisibilityTracker.ts | 186 +++ 8 files changed, 1494 insertions(+), 1045 deletions(-) create mode 100644 packages/doenetml-worker-javascript/src/AutoSubmitManager.ts create mode 100644 packages/doenetml-worker-javascript/src/DiagnosticsManager.ts create mode 100644 packages/doenetml-worker-javascript/src/NavigationHandler.ts create mode 100644 packages/doenetml-worker-javascript/src/ResolverAdapter.ts create mode 100644 packages/doenetml-worker-javascript/src/StatePersistence.ts create mode 100644 packages/doenetml-worker-javascript/src/StateVariableNameResolver.ts create mode 100644 packages/doenetml-worker-javascript/src/VisibilityTracker.ts diff --git a/packages/doenetml-worker-javascript/src/AutoSubmitManager.ts b/packages/doenetml-worker-javascript/src/AutoSubmitManager.ts new file mode 100644 index 000000000..e2de4e565 --- /dev/null +++ b/packages/doenetml-worker-javascript/src/AutoSubmitManager.ts @@ -0,0 +1,60 @@ +/** + * Owns the debounced auto-submit-answer queue. When state changes record an + * answer that should be submitted, the manager batches them and dispatches + * `submitAnswer` actions after a short debounce. + * + * Holds a back-reference to Core to look up components and dispatch actions. + */ +export class AutoSubmitManager { + core: any; + answersToSubmit: number[]; + submitAnswersTimeout: ReturnType | null; + + constructor({ core }: { core: any }) { + this.core = core; + this.answersToSubmit = []; + this.submitAnswersTimeout = null; + } + + recordAnswer(componentIdx: number): void { + if (!this.answersToSubmit.includes(componentIdx)) { + this.answersToSubmit.push(componentIdx); + } + + if (this.submitAnswersTimeout !== null) { + clearTimeout(this.submitAnswersTimeout); + } + + //Debounce the submit answers + this.submitAnswersTimeout = setTimeout(() => { + this.submitNow(); + }, 1000); + } + + async submitNow(): Promise { + let toSubmit = this.answersToSubmit; + this.answersToSubmit = []; + for (let componentIdx of toSubmit) { + let component = this.core._components[componentIdx]; + + if (component.actions.submitAnswer) { + await this.core.requestAction({ + componentIdx, + actionName: "submitAnswer", + }); + } + } + } + + /** + * Cancel any pending debounce and submit immediately. Used during + * `Core.terminate()` to flush queued answers before shutdown. + */ + async flush(): Promise { + if (this.submitAnswersTimeout !== null) { + clearTimeout(this.submitAnswersTimeout); + this.submitAnswersTimeout = null; + await this.submitNow(); + } + } +} diff --git a/packages/doenetml-worker-javascript/src/Core.js b/packages/doenetml-worker-javascript/src/Core.js index 0b3835946..68be61537 100644 --- a/packages/doenetml-worker-javascript/src/Core.js +++ b/packages/doenetml-worker-javascript/src/Core.js @@ -9,7 +9,6 @@ import { assignDoenetMLRange, findAllNewlines, flattenDeep, - data_format_version, } from "@doenet/utils"; import { convertToErrorComponent } from "./utils/dast/errors"; import { gatherVariantComponents, getNumVariants } from "./utils/variants"; @@ -28,21 +27,28 @@ import { unwrapSource, } from "./utils/dast/convertNormalizedDast"; import { DependencyHandler } from "./Dependencies"; +import { AutoSubmitManager } from "./AutoSubmitManager"; +import { DiagnosticsManager } from "./DiagnosticsManager"; +import { NavigationHandler } from "./NavigationHandler"; +import { ResolverAdapter } from "./ResolverAdapter"; +import { StatePersistence } from "./StatePersistence"; +import { VisibilityTracker } from "./VisibilityTracker"; +import { + findCaseInsensitiveMatches as resolveCaseInsensitiveMatches, + matchPublicStateVariables as resolveMatchPublicStateVariables, + substituteAliases as resolveSubstituteAliases, + publicCaseInsensitiveAliasSubstitutions as resolvePublicCaseInsensitiveAliasSubstitutions, + checkIfArrayEntry as resolveCheckIfArrayEntry, +} from "./StateVariableNameResolver"; import { returnDefaultArrayVarNameFromPropIndex, returnDefaultGetArrayKeysFromVarName, } from "./utils/stateVariables"; -import { set as idb_set } from "idb-keyval"; import { createComponentIndicesFromSerializedChildren, createNewComponentIndices, extractCreateComponentIdxMapping, } from "./utils/componentIndices"; -import { - addNodesToFlatFragment, - getEffectiveComponentIdx, -} from "./utils/resolver"; - // string to componentClass: this.componentInfoObjects.allComponentClasses["string"] // componentClass to string: componentClass.componentType @@ -112,23 +118,10 @@ export default class Core { this.cid = cid; - /** @type {({ type: "error"|"warning"|"info", message: string, position?: any, sourceDoc?: number } | { type: "accessibility", level: 1|2, message: string, position?: any, sourceDoc?: number })[]} */ - this.diagnostics = preliminaryDiagnostics - // Note: we ignore preliminary errors, as we'll gather those from the dast when processing it. - .filter((diagnostic) => diagnostic.type !== "error") - .map((diagnostic) => { - this.assertDiagnosticIsValid(diagnostic); - - return { - type: diagnostic.type, - ...(diagnostic.type === "accessibility" - ? { level: diagnostic.level } - : {}), - message: diagnostic.message, - position: diagnostic.position, - sourceDoc: diagnostic.sourceDoc, - }; - }); + this.diagnosticsManager = new DiagnosticsManager({ + core: this, + preliminaryDiagnostics, + }); this.numerics = new Numerics(); // this.flags = new Proxy(flags, readOnlyProxyHandler); //components shouldn't modify flags @@ -191,8 +184,6 @@ export default class Core { stateVariablesToEvaluate: [], }; - this.hasPendingDiagnostics = true; - this.cumulativeStateVariableChanges = JSON.parse( JSON.stringify(stateVariableChanges, serializedComponentsReplacer), serializedComponentsReviver, @@ -201,17 +192,10 @@ export default class Core { this.requestedVariantIndex = requestedVariantIndex; this.requestedVariant = requestedVariant; - this.visibilityInfo = { - componentsCurrentlyVisible: {}, - infoToSend: {}, - timeLastSent: new Date(), - saveDelay: 60000, - saveTimerId: null, - suspendDelay: 3 * 60000, - suspendTimerId: null, - suspended: false, - documentHasBeenVisible: false, - }; + this.visibilityTracker = new VisibilityTracker({ core: this }); + this.autoSubmitManager = new AutoSubmitManager({ core: this }); + this.navigationHandler = new NavigationHandler({ core: this }); + this.resolverAdapter = new ResolverAdapter({ core: this }); // console.time('serialize doenetML'); @@ -262,7 +246,7 @@ export default class Core { this.essentialValuesSavedInDefinition = {}; - this.saveStateToDBTimerId = null; + this.statePersistence = new StatePersistence({ core: this }); // rendererState the current state of each renderer, keyed by componentIdx this.rendererState = {}; @@ -475,112 +459,37 @@ export default class Core { this.updateRenderersCallback({ ...args, init, diagnostics }); } - /** - * Get pending diagnostics and reset the pending flag. - * Automatically trims the diagnostics array to prevent unbounded memory growth. - * - * @returns {Object} Object containing the current diagnostics array - * @note Diagnostics older than the 1000 most recent are discarded to manage memory - */ - getDiagnostics() { - // Keep only the last 1000 diagnostics to avoid unbounded memory growth. - // Once the limit is exceeded, older diagnostics are discarded. - // This ensures the codebase doesn't accumulate large numbers of stale messages. - const MAX_DIAGNOSTICS = 1000; - this.diagnostics = this.diagnostics.slice(-MAX_DIAGNOSTICS); + // Diagnostic state and helpers live in `this.diagnosticsManager` + // (see DiagnosticsManager.ts). The accessors and methods below preserve + // the public surface (`core.diagnostics`, `core.hasPendingDiagnostics`, + // `core.addDiagnostic`, etc.) by delegating through. - this.hasPendingDiagnostics = false; - - return { diagnostics: this.diagnostics }; + get diagnostics() { + return this.diagnosticsManager.diagnostics; } - /** - * Add a diagnostic record to `this.diagnostics`, deduplicating by - * type + message + source location. - * - * @returns {boolean} `true` if a new entry was added, `false` if deduped. - */ - assertDiagnosticIsValid({ type, level }) { - if (!["error", "warning", "info", "accessibility"].includes(type)) { - throw Error("Invalid diagnostic type"); - } - - if (type === "accessibility") { - if (level === undefined) { - throw Error("Missing accessibility diagnostic level"); - } - - if (![1, 2].includes(level)) { - throw Error("Invalid accessibility diagnostic level"); - } - } + get hasPendingDiagnostics() { + return this.diagnosticsManager.hasPendingDiagnostics; } - addDiagnostic({ type, level, message, position, sourceDoc }) { - const sameLocation = (pointA, pointB) => - (pointA?.offset ?? undefined) === (pointB?.offset ?? undefined) && - (pointA?.line ?? undefined) === (pointB?.line ?? undefined) && - (pointA?.column ?? undefined) === (pointB?.column ?? undefined); - - const haveSamePosition = (warningPosition, newPosition) => { - if (warningPosition === undefined || newPosition === undefined) { - return warningPosition === newPosition; - } - - return ( - sameLocation(warningPosition.start, newPosition.start) && - sameLocation(warningPosition.end, newPosition.end) - ); - }; - - this.assertDiagnosticIsValid({ type, level }); - - const alreadyHaveDiagnostic = this.diagnostics.some( - (diagnostic) => - diagnostic.type === type && - (type === "accessibility" - ? diagnostic.level === level - : true) && - diagnostic.message === message && - diagnostic.sourceDoc === sourceDoc && - haveSamePosition(diagnostic.position, position), - ); + set hasPendingDiagnostics(value) { + this.diagnosticsManager.hasPendingDiagnostics = value; + } - if (alreadyHaveDiagnostic) { - return false; - } + getDiagnostics() { + return this.diagnosticsManager.getDiagnostics(); + } - this.diagnostics.push({ - type, - ...(type === "accessibility" ? { level } : {}), - message, - position, - sourceDoc, - }); + assertDiagnosticIsValid(diagnostic) { + this.diagnosticsManager.assertDiagnosticIsValid(diagnostic); + } - this.hasPendingDiagnostics = true; - return true; + addDiagnostic(diagnostic) { + return this.diagnosticsManager.addDiagnostic(diagnostic); } - /** - * Find the nearest available source position/sourceDoc for a component, - * walking up ancestors when the component itself has no position. - */ getSourceLocationForComponent(component) { - let position = component.position; - let sourceDoc = component.sourceDoc; - let comp = component; - - while (position === undefined) { - if (!(comp.parentIdx > 0)) { - break; - } - comp = this._components[comp.parentIdx]; - position = comp.position; - sourceDoc = comp.sourceDoc; - } - - return { position, sourceDoc }; + return this.diagnosticsManager.getSourceLocationForComponent(component); } async addComponents({ @@ -2726,384 +2635,31 @@ export default class Core { return { success: true, compositesExpanded: [component.componentIdx] }; } - async addReplacementsToResolver({ - serializedReplacements, - component, - updateOldReplacementsStart, - updateOldReplacementsEnd, - blankStringReplacements, - }) { - if (component.constructor.replacementsAlreadyInResolver) { - return; - } + // Resolver adapter methods live in `this.resolverAdapter` + // (see ResolverAdapter.ts). The methods below preserve the public surface + // (`core.addReplacementsToResolver`, etc.) by delegating through. - const { parentIdx, indexResolution } = - await this.determineParentAndIndexResolutionForResolver({ - component, - updateOldReplacementsStart, - updateOldReplacementsEnd, - blankStringReplacements, - }); - - // If `createComponentIdx` was specified, the one replacement is already in the resolver, - // so we just add its children and attribute components/references. - // Otherwise add all replacements. - const fragmentChildren = []; - let parentSourceSequence = null; - if (component.attributes.createComponentIdx != null) { - if (serializedReplacements[0]?.children) { - fragmentChildren.push(...serializedReplacements[0].children); - } - for (const attrName in serializedReplacements[0]?.attributes) { - const attribute = - serializedReplacements[0].attributes[attrName]; - if (attribute.type === "component") { - fragmentChildren.push(attribute.component); - } else if (attribute.type === "references") { - fragmentChildren.push(...attribute.references); - } - } - - // if the replacement that is the fragment parent has a source sequence, - // then add that as the `parentSourceSequence` of the flat fragment - let sourceSequence = - serializedReplacements[0]?.attributes["source:sequence"]; - if (sourceSequence) { - parentSourceSequence = { - type: "attribute", - name: "source:sequence", - parent: component.attributes.createComponentIdx.primitive - .number, - children: sourceSequence.children.filter( - (child) => typeof child === "string", - ), - sourceDoc: sourceSequence.sourceDoc, - }; - } - } else { - fragmentChildren.push(...serializedReplacements); - } - - // We add all the parent's descendants to the resolver - const flatFragment = { - children: fragmentChildren.map((child) => - typeof child === "string" - ? child - : getEffectiveComponentIdx(child), - ), - nodes: [], - parentIdx, - parentSourceSequence, - idxMap: {}, - }; - - addNodesToFlatFragment({ - flatFragment, - serializedComponents: fragmentChildren, - parentIdx, - }); - - if ( - (flatFragment.nodes.length > 0 || indexResolution !== "None") && - this.addNodesToResolver - ) { - // console.log("add nodes to resolver", { - // flatFragment, - // indexResolution, - // }); - this.addNodesToResolver(flatFragment, indexResolution); - - this.rootNames = this.calculateRootNames?.().names; - - let indexParent = - indexResolution.ReplaceAll?.parent ?? - indexResolution.ReplaceRange?.parent ?? - null; - - if ( - indexParent !== null && - indexParent !== component.componentIdx - ) { - const indexParentComposite = this._components[indexParent]; - - if (indexParentComposite) { - await this.dependencies.addBlockersFromChangedReplacements( - indexParentComposite, - ); - } - } - } + async addReplacementsToResolver(args) { + return this.resolverAdapter.addReplacementsToResolver(args); } - async determineParentAndIndexResolutionForResolver({ - component, - updateOldReplacementsStart, - updateOldReplacementsEnd, - blankStringReplacements, - }) { - // If the composite was created as a child for a list, - // then the parent for resolving names is that list (the parent of the resolver). - // If `createComponentIdx` was specified, then that should be the parent for resolving names. - // Else, the composite should be the parent for resolving names. - - let update_start = updateOldReplacementsStart; - let update_end = updateOldReplacementsEnd; - - if ( - updateOldReplacementsStart !== undefined && - updateOldReplacementsEnd !== undefined - ) { - // We are replacing a range of replacement, but these include blank strings. - // Adjust the range to ignore blank strings - for (const [ - i, - isBlankString, - ] of blankStringReplacements.entries()) { - if (i >= updateOldReplacementsEnd) { - break; - } - if (isBlankString) { - update_end--; - if (i < updateOldReplacementsStart) { - update_start--; - } - } - } - } - - let parentIdx; - - let indexResolution = "None"; - - if (component.doenetAttributes.forList) { - // Don't add index resolutions in this case, - // we're just adding to the children of the list, not the replacements of the list - parentIdx = component.parentIdx; - } else if (component.attributes.createComponentIdx?.primitive) { - // If `createComponentIdx` is set, then we have a copy component created from an `extend` attribute. - // That component is already in the resolver so will be the parent of the fragment added to the browser. - parentIdx = - component.attributes.createComponentIdx?.primitive.value; - - // If the component type of that parent, specified by `createComponentOfType`, is a composite, - // then it could have an index specified, so we add an index resolution - if ( - component.attributes.createComponentOfType?.primitive && - this.componentInfoObjects.isCompositeComponent({ - componentType: - component.attributes.createComponentOfType.primitive - .value, - includeNonStandard: true, - }) - ) { - indexResolution = { ReplaceAll: { parent: parentIdx } }; - - if (update_start !== undefined && update_end !== undefined) { - const parent = this._components[parentIdx]; - - indexResolution = { - ReplaceRange: { - parent: parentIdx, - range: { start: update_start, end: update_end }, - }, - }; - } - } - } else if (component.componentType === "_copy") { - // If we have a copy that wasn't from an extend, then it was from a reference. - // Although references don't have names that can be - // Copy components are typically not part of the resolver structure and generally skipped. - // Since we don't allow direct authoring of copy components, - // they should occur only from references - - // determine if is a replacement of another type of composite - let copyComponent = component; - parentIdx = component.componentIdx; - - while (copyComponent.replacementOf) { - if (copyComponent.replacementOf.componentType === "_copy") { - copyComponent = copyComponent.replacementOf; - continue; - } else { - break; - } - } - - // now we have a copyComponent that is not a replacement of a copy - if (copyComponent.replacementOf) { - const indexParent = copyComponent.replacementOf; - - // determine where the replacement will end up being spliced in - - let start_idx, end_idx; - - async function calcStartEndIdx(replacements) { - let nonWithheldReplacements = []; - for (const repl of replacements) { - if ( - typeof repl === "string" || - !(await repl.stateValues - .isInactiveCompositeReplacement) - ) { - nonWithheldReplacements.push(repl); - } - } - - const nonBlankStringReplacements = - nonWithheldReplacements.filter( - (x) => typeof x !== "string" || x.trim() !== "", - ); - const replacementsWithoutExpandedCopies = []; - - let i = 0; - - for (const repl of nonBlankStringReplacements) { - if (repl.componentType == "_copy") { - if (!repl.isExpanded) { - if ( - repl.componentIdx === - copyComponent.componentIdx - ) { - start_idx = i; - end_idx = i + 1; - } - replacementsWithoutExpandedCopies.push(repl); - i++; - } else { - let replReplacements = repl.replacements; - if (repl.replacementsToWithhold) { - replReplacements = replReplacements.slice( - 0, - replReplacements.length - - repl.replacementsToWithhold, - ); - } - - const newReplacements = - await calcStartEndIdx(replReplacements); - const n = newReplacements.length; - - if ( - repl.componentIdx === - copyComponent.componentIdx - ) { - if ( - update_start !== undefined && - update_end !== undefined - ) { - start_idx = i + update_start; - end_idx = i + update_end; - } else { - start_idx = i; - end_idx = i + n; - } - } - - replacementsWithoutExpandedCopies.push( - ...newReplacements, - ); - i += n; - } - } else { - replacementsWithoutExpandedCopies.push(repl); - i++; - } - } - - return replacementsWithoutExpandedCopies; - } - - await calcStartEndIdx(indexParent.replacements); - - if (start_idx !== undefined && end_idx !== undefined) { - indexResolution = { - ReplaceRange: { - parent: indexParent.componentIdx, - range: { start: start_idx, end: end_idx }, - }, - }; - } else { - // if the copy was not found as a replacement of the composite, - // then it wasn't a top-level replacement and it doesn't affect the composite's index resolution - indexResolution = "None"; - } - } else { - parentIdx = copyComponent.componentIdx; - indexResolution = { ReplaceAll: { parent: parentIdx } }; - } - } else { - parentIdx = component.componentIdx; - - if ( - this.componentInfoObjects.isCompositeComponent({ - componentType: component.componentType, - includeNonStandard: true, - }) - ) { - if (update_start !== undefined && update_end !== undefined) { - indexResolution = { - ReplaceRange: { - parent: parentIdx, - range: { start: update_start, end: update_end }, - }, - }; - } else { - indexResolution = { ReplaceAll: { parent: parentIdx } }; - } - } - } - - return { parentIdx, indexResolution }; + async determineParentAndIndexResolutionForResolver(args) { + return this.resolverAdapter.determineParentAndIndexResolutionForResolver( + args, + ); } addComponentsToResolver(components, parentIdx) { - const flatFragment = { - children: components.map((child) => - typeof child === "string" - ? child - : getEffectiveComponentIdx(child), - ), - nodes: [], - parentIdx, - idxMap: {}, - }; - - addNodesToFlatFragment({ - flatFragment, - serializedComponents: components, + return this.resolverAdapter.addComponentsToResolver( + components, parentIdx, - }); - - // console.log("add nodes from components to resolver", { - // flatFragment, - // }); - - if (this.addNodesToResolver) { - this.addNodesToResolver(flatFragment, "None"); - - this.rootNames = this.calculateRootNames?.().names; - } + ); } - gatherDiagnosticsAndAssignDoenetMLRange({ - components, - diagnostics, - position, - sourceDoc, - overwriteDoenetMLRange = false, - }) { - assignDoenetMLRange( - components, - position, - sourceDoc, - overwriteDoenetMLRange, + gatherDiagnosticsAndAssignDoenetMLRange(args) { + return this.resolverAdapter.gatherDiagnosticsAndAssignDoenetMLRange( + args, ); - assignDoenetMLRange(diagnostics, position, sourceDoc); - - // Add all diagnostics, preserving their existing type field - for (const diagnostic of diagnostics) { - this.addDiagnostic(diagnostic); - } } async expandShadowingComposite(component) { @@ -8186,241 +7742,49 @@ export default class Core { } } - findCaseInsensitiveMatches({ stateVariables, componentClass }) { - let stateVarInfo = - this.componentInfoObjects.stateVariableInfo[ - componentClass.componentType - ]; - - let newVariables = []; + // The five state-variable name-resolution helpers below live as pure + // functions in StateVariableNameResolver.ts. The wrappers preserve the + // public surface (`core.findCaseInsensitiveMatches`, etc., plus the + // by-reference passes used in composite sugar functions) by injecting + // `componentInfoObjects` and delegating. - for (let stateVariable of stateVariables) { - let foundMatch = false; - - let lowerCaseVarName = stateVariable.toLowerCase(); - - for (let varName in stateVarInfo.stateVariableDescriptions) { - if (lowerCaseVarName === varName.toLowerCase()) { - foundMatch = true; - newVariables.push(varName); - break; - } - } - - if (foundMatch) { - continue; - } - - let isArraySize = false; - let lowerCaseNameMinusSize = lowerCaseVarName; - if (lowerCaseVarName.substring(0, 13) === "__array_size_") { - isArraySize = true; - lowerCaseNameMinusSize = lowerCaseVarName.substring(13); - } - - for (let aliasName in stateVarInfo.aliases) { - if (lowerCaseNameMinusSize === aliasName.toLowerCase()) { - // don't substitute alias here, just fix case - if (isArraySize) { - aliasName = "__array_size_" + aliasName; - } - newVariables.push(aliasName); - foundMatch = true; - break; - } - } - if (foundMatch) { - continue; - } - - let arrayEntryPrefixesLongestToShortest = Object.keys( - stateVarInfo.arrayEntryPrefixes, - ).sort((a, b) => b.length - a.length); - for (let prefix of arrayEntryPrefixesLongestToShortest) { - if ( - lowerCaseVarName.substring(0, prefix.length) === - prefix.toLowerCase() - ) { - // TODO: the varEnding is still a case-senstitive match - // Should we require that getArrayKeysFromVarName have - // a case-insensitive mode? - let arrayVariableName = - stateVarInfo.arrayEntryPrefixes[prefix] - .arrayVariableName; - let arrayStateVarDescription = - stateVarInfo.stateVariableDescriptions[ - arrayVariableName - ]; - let arrayKeys = - arrayStateVarDescription.getArrayKeysFromVarName({ - arrayEntryPrefix: prefix, - varEnding: stateVariable.substring(prefix.length), - numDimensions: - arrayStateVarDescription.numDimensions, - }); - if (arrayKeys.length > 0) { - let newVarName = - prefix + lowerCaseVarName.substring(prefix.length); - foundMatch = true; - newVariables.push(newVarName); - break; - } - } - } - - if (foundMatch) { - continue; - } - - // no match, so don't alter - newVariables.push(stateVariable); - } - - return newVariables; + findCaseInsensitiveMatches({ stateVariables, componentClass }) { + return resolveCaseInsensitiveMatches({ + stateVariables, + componentClass, + componentInfoObjects: this.componentInfoObjects, + }); } matchPublicStateVariables({ stateVariables, componentClass }) { - let stateVarInfo = - this.componentInfoObjects.publicStateVariableInfo[ - componentClass.componentType - ]; - - let newVariables = []; - - for (let stateVariable of stateVariables) { - if (stateVariable in stateVarInfo.stateVariableDescriptions) { - // found public - newVariables.push(stateVariable); - continue; - } - - let varName = stateVariable; - - if (varName in stateVarInfo.aliases) { - varName = stateVarInfo.aliases[varName]; - - // check again to see if alias is public - if (varName in stateVarInfo.stateVariableDescriptions) { - // found public - newVariables.push(varName); - continue; - } - } - - let foundMatch = false; - - let arrayEntryPrefixesLongestToShortest = Object.keys( - stateVarInfo.arrayEntryPrefixes, - ).sort((a, b) => b.length - a.length); - for (let prefix of arrayEntryPrefixesLongestToShortest) { - if (varName.substring(0, prefix.length) === prefix) { - let arrayVariableName = - stateVarInfo.arrayEntryPrefixes[prefix] - .arrayVariableName; - let arrayStateVarDescription = - stateVarInfo.stateVariableDescriptions[ - arrayVariableName - ]; - let arrayKeys = - arrayStateVarDescription.getArrayKeysFromVarName({ - arrayEntryPrefix: prefix, - varEnding: varName.substring(prefix.length), - numDimensions: - arrayStateVarDescription.numDimensions, - }); - if (arrayKeys.length > 0) { - foundMatch = true; - break; - } - } - } - - if (foundMatch) { - newVariables.push(stateVariable); - } else { - // no match, so make it a name that won't match - newVariables.push("__not_public_" + stateVariable); - } - } - - return newVariables; + return resolveMatchPublicStateVariables({ + stateVariables, + componentClass, + componentInfoObjects: this.componentInfoObjects, + }); } substituteAliases({ stateVariables, componentClass }) { - let newVariables = []; - - let stateVarInfo = - this.componentInfoObjects.stateVariableInfo[ - componentClass.componentType - ]; - - for (let stateVariable of stateVariables) { - let isArraySize = false; - if (stateVariable.substring(0, 13) === "__array_size_") { - isArraySize = true; - stateVariable = stateVariable.substring(13); - } - stateVariable = - stateVariable in stateVarInfo.aliases - ? stateVarInfo.aliases[stateVariable] - : stateVariable; - if (isArraySize) { - stateVariable = "__array_size_" + stateVariable; - } - newVariables.push(stateVariable); - } - - return newVariables; + return resolveSubstituteAliases({ + stateVariables, + componentClass, + componentInfoObjects: this.componentInfoObjects, + }); } publicCaseInsensitiveAliasSubstitutions({ stateVariables, componentClass, }) { - let mappedVarNames = this.findCaseInsensitiveMatches({ + return resolvePublicCaseInsensitiveAliasSubstitutions({ stateVariables, componentClass, + componentInfoObjects: this.componentInfoObjects, }); - - mappedVarNames = this.matchPublicStateVariables({ - stateVariables: mappedVarNames, - componentClass, - }); - - mappedVarNames = this.substituteAliases({ - stateVariables: mappedVarNames, - componentClass, - }); - - return mappedVarNames; } checkIfArrayEntry({ stateVariable, component }) { - // check if stateVariable begins when an arrayEntry - for (let arrayEntryPrefix in component.arrayEntryPrefixes) { - if ( - stateVariable.substring(0, arrayEntryPrefix.length) === - arrayEntryPrefix - ) { - let arrayVariableName = - component.arrayEntryPrefixes[arrayEntryPrefix]; - let arrayStateVarObj = component.state[arrayVariableName]; - let arrayKeys = arrayStateVarObj.getArrayKeysFromVarName({ - arrayEntryPrefix, - varEnding: stateVariable.substring(arrayEntryPrefix.length), - numDimensions: arrayStateVarObj.numDimensions, - }); - if (arrayKeys.length > 0) { - return { - isArrayEntry: true, - arrayVariableName, - arrayEntryPrefix, - }; - } - } - } - - return { isArrayEntry: false }; + return resolveCheckIfArrayEntry({ stateVariable, component }); } async createFromArrayEntry({ @@ -9955,47 +9319,9 @@ export default class Core { } removeComponentsFromResolver(componentsToRemove) { - if (componentsToRemove.length === 0) { - return; - } - - const flatElements = componentsToRemove.map((comp) => { - let flatElement = { - type: "element", - name: comp.componentType, - parent: comp.parentIdx, - children: [], - attributes: [], - idx: comp.componentIdx, - }; - - if (comp.attributes.createComponentName && !comp.isExpanded) { - flatElement.attributes.push({ - type: "attribute", - name: "name", - parent: comp.parentIdx, - children: [ - comp.attributes.createComponentName.primitive.value, - ], - }); - } else if (comp.attributes.name) { - flatElement.attributes.push({ - type: "attribute", - name: "name", - parent: comp.parentIdx, - children: [comp.attributes.name.primitive.value], - }); - } - return flatElement; - }); - - if (this.deleteNodesFromResolver) { - this.deleteNodesFromResolver({ - nodes: flatElements, - }); - - this.rootNames = this.calculateRootNames?.().names; - } + return this.resolverAdapter.removeComponentsFromResolver( + componentsToRemove, + ); } determineComponentsToDelete({ @@ -11847,12 +11173,8 @@ export default class Core { alreadySaved = true; } if (!alreadySaved && !doNotSave) { - clearTimeout(this.saveDocStateTimeoutID); - //Debounce the save to localstorage and then to DB with a throttle - this.saveDocStateTimeoutID = setTimeout(() => { - this.saveState(); - }, 1000); + this.statePersistence.scheduleSave(1000); } // evaluate componentCreditAchieved so that will be fresh @@ -11954,140 +11276,29 @@ export default class Core { this.sendEvent(payload); } - processVisibilityChangedEvent(event) { - let componentIdx = event.object.componentIdx; - let isVisible = event.result.isVisible; - - if (isVisible) { - if (!this.visibilityInfo.componentsCurrentlyVisible[componentIdx]) { - this.visibilityInfo.componentsCurrentlyVisible[componentIdx] = - new Date(); - } - if (componentIdx === this.documentIdx) { - if (!this.visibilityInfo.documentHasBeenVisible) { - this.visibilityInfo.documentHasBeenVisible = true; - this.onDocumentFirstVisible(); - } - } - } else { - let begin = - this.visibilityInfo.componentsCurrentlyVisible[componentIdx]; - if (begin) { - delete this.visibilityInfo.componentsCurrentlyVisible[ - componentIdx - ]; + // Visibility tracking lives in `this.visibilityTracker` + // (see VisibilityTracker.ts). The accessor and methods below preserve + // the public surface (`core.visibilityInfo`, `core.processVisibilityChangedEvent`, + // etc.) by delegating through. - let timeInSeconds = - (new Date() - - Math.max(begin, this.visibilityInfo.timeLastSent)) / - 1000; + get visibilityInfo() { + return this.visibilityTracker.info; + } - if (this.visibilityInfo.infoToSend[componentIdx]) { - this.visibilityInfo.infoToSend[componentIdx] += - timeInSeconds; - } else { - this.visibilityInfo.infoToSend[componentIdx] = - timeInSeconds; - } - } - } + processVisibilityChangedEvent(event) { + return this.visibilityTracker.processVisibilityChangedEvent(event); } sendVisibilityChangedEvents() { - let infoToSend = { ...this.visibilityInfo.infoToSend }; - this.visibilityInfo.infoToSend = {}; - let timeLastSent = this.visibilityInfo.timeLastSent; - this.visibilityInfo.timeLastSent = new Date(); - let currentVisible = { - ...this.visibilityInfo.componentsCurrentlyVisible, - }; - - for (const componentIdxStr in currentVisible) { - let timeInSeconds = - (this.visibilityInfo.timeLastSent - - Math.max(timeLastSent, currentVisible[componentIdxStr])) / - 1000; - if (infoToSend[componentIdxStr]) { - infoToSend[componentIdxStr] += timeInSeconds; - } else { - infoToSend[componentIdxStr] = timeInSeconds; - } - } - - for (const componentIdxStr in infoToSend) { - infoToSend[componentIdxStr] = Math.round( - infoToSend[componentIdxStr], - ); - if (!infoToSend[componentIdxStr]) { - // delete if rounded down to zero - delete infoToSend[componentIdxStr]; - } - } - - let promise; - - if (Object.keys(infoToSend).length > 0) { - let event = { - object: { - componentIdx: this.documentIdx, - componentType: "document", - }, - verb: "isVisible", - result: infoToSend, - }; - - promise = new Promise((resolve, reject) => { - this.processQueue.push({ - type: "recordEvent", - event, - resolve, - reject, - }); - - if (!this.processing) { - this.processing = true; - this.executeProcesses(); - } - }); - } - - if (!this.visibilityInfo.suspended) { - clearTimeout(this.visibilityInfo.saveTimerId); - this.visibilityInfo.saveTimerId = setTimeout( - this.sendVisibilityChangedEvents.bind(this), - this.visibilityInfo.saveDelay, - ); - } - - return promise; + return this.visibilityTracker.sendVisibilityChangedEvents(); } async suspendVisibilityMeasuring() { - clearTimeout(this.visibilityInfo.saveTimerId); - clearTimeout(this.visibilityInfo.suspendTimerId); - if (!this.visibilityInfo.suspended) { - this.visibilityInfo.suspended = true; - await this.sendVisibilityChangedEvents(); - } + return this.visibilityTracker.suspendVisibilityMeasuring(); } resumeVisibilityMeasuring() { - if (this.visibilityInfo.suspended) { - // restart visibility measuring - this.visibilityInfo.suspended = false; - this.visibilityInfo.timeLastSent = new Date(); - clearTimeout(this.visibilityInfo.saveTimerId); - this.visibilityInfo.saveTimerId = setTimeout( - this.sendVisibilityChangedEvents.bind(this), - this.visibilityInfo.saveDelay, - ); - } - - clearTimeout(this.visibilityInfo.suspendTimerId); - this.visibilityInfo.suspendTimerId = setTimeout( - this.suspendVisibilityMeasuring.bind(this), - this.visibilityInfo.suspendDelay, - ); + return this.visibilityTracker.resumeVisibilityMeasuring(); } async executeUpdateStateVariables(newStateVariableValues) { @@ -13415,104 +12626,21 @@ export default class Core { } } + // State persistence (save to localStorage / database) lives in + // `this.statePersistence` (see StatePersistence.ts). The methods below + // preserve the public surface (`core.saveImmediately`, `core.saveState`, + // `core.saveChangesToDatabase`) by delegating through. + async saveImmediately() { - if (this.saveDocStateTimeoutID) { - // if in debounce to save doc to local storage - // then immediate save to local storage - // and override timeout to save to database - clearTimeout(this.saveDocStateTimeoutID); - await this.saveState(true); - } else { - // else override timeout to save any pending changes to database - await this.saveChangesToDatabase(true); - } + return this.statePersistence.saveImmediately(); } async saveState(overrideThrottle = false, onSubmission = false) { - this.saveDocStateTimeoutID = null; - - if (!this.flags.allowSaveState && !this.flags.allowLocalState) { - return; - } - - let coreStateString = JSON.stringify( - this.cumulativeStateVariableChanges, - serializedComponentsReplacer, - ); - let rendererStateString = null; - - if (this.flags.saveRendererState) { - rendererStateString = JSON.stringify( - this.rendererState, - serializedComponentsReplacer, - ); - } - - if (this.flags.allowLocalState) { - await idb_set( - `${this.activityId}|${this.docId}|${this.attemptNumber}|${this.cid}`, - { - data_format_version, - coreState: coreStateString, - rendererState: rendererStateString, - coreInfo: this.coreInfoString, - }, - ); - } - - if (!this.flags.allowSaveState) { - return; - } - - this.docStateToBeSavedToDatabase = { - cid: this.cid, - coreInfo: this.coreInfoString, - coreState: coreStateString, - rendererState: rendererStateString, - initializeCounters: this.initializeCounters, - docId: this.docId, - attemptNumber: this.attemptNumber, - activityId: this.activityId, - onSubmission, - }; - - // mark presence of changes - // so that next call to saveChangesToDatabase will save changes - this.changesToBeSaved = true; - - // if not currently in throttle, save changes to database - await this.saveChangesToDatabase(overrideThrottle); + return this.statePersistence.saveState(overrideThrottle, onSubmission); } async saveChangesToDatabase(overrideThrottle) { - // throttle save to database at 60 seconds - - if (!this.changesToBeSaved) { - return; - } - - if (this.saveStateToDBTimerId !== null) { - if (overrideThrottle) { - clearTimeout(this.saveStateToDBTimerId); - } else { - return; - } - } - - this.changesToBeSaved = false; - - // check for changes again after 60 seconds - this.saveStateToDBTimerId = setTimeout(() => { - this.saveStateToDBTimerId = null; - this.saveChangesToDatabase(); - }, 60000); - - this.reportScoreAndStateCallback({ - state: { ...this.docStateToBeSavedToDatabase }, - score: await this.document.stateValues.creditAchieved, - }); - - return; + return this.statePersistence.saveChangesToDatabase(overrideThrottle); } /** @@ -13546,40 +12674,11 @@ export default class Core { } } - async handleNavigatingToComponent({ componentIdx, hash }) { - let component = this._components[componentIdx]; - if (component) { - let componentAndAncestors = [ - componentIdx, - ...component.ancestors.map((x) => x.componentIdx), - ]; - let openedParent = false; - for (let cIdx of componentAndAncestors) { - let comp = this._components[cIdx]; - if (comp.actions?.revealSection) { - let isOpen = await comp.stateValues.open; + // Navigation methods delegate to `this.navigationHandler` + // (see NavigationHandler.ts). - if (isOpen === false) { - await this.performAction({ - componentIdx: cIdx, - actionName: "revealSection", - }); - if (cIdx !== componentIdx) { - openedParent = true; - } - } - } - } - if (openedParent) { - // If just opened parent, then we couldn't have navigated to target yet - // as the target didn't exist in the DOM when the parent was closed. - // Navigate to the specified hash now. - postMessage({ - messageType: "navigateToHash", - args: { hash }, - }); - } - } + async handleNavigatingToComponent(args) { + return this.navigationHandler.handleNavigatingToComponent(args); } async terminate() { @@ -13592,10 +12691,7 @@ export default class Core { // suspend visibility measuring so that remaining times collected are saved await this.suspendVisibilityMeasuring(); - if (this.submitAnswersTimeout) { - clearTimeout(this.submitAnswersTimeout); - await this.autoSubmitAnswers(); - } + await this.autoSubmitManager.flush(); this.stopProcessingRequests = true; @@ -13611,36 +12707,16 @@ export default class Core { await this.saveImmediately(); } - recordAnswerToAutoSubmit(componentIdx) { - if (!this.answersToSubmit) { - this.answersToSubmit = []; - } - - if (!this.answersToSubmit.includes(componentIdx)) { - this.answersToSubmit.push(componentIdx); - } + // Auto-submit-answer queue lives in `this.autoSubmitManager` + // (see AutoSubmitManager.ts). The methods below preserve the public surface + // (`core.recordAnswerToAutoSubmit`, `core.autoSubmitAnswers`) by delegating. - clearTimeout(this.submitAnswersTimeout); - - //Debounce the submit answers - this.submitAnswersTimeout = setTimeout(() => { - this.autoSubmitAnswers(); - }, 1000); + recordAnswerToAutoSubmit(componentIdx) { + this.autoSubmitManager.recordAnswer(componentIdx); } async autoSubmitAnswers() { - let toSubmit = this.answersToSubmit; - this.answersToSubmit = []; - for (let componentIdx of toSubmit) { - let component = this._components[componentIdx]; - - if (component.actions.submitAnswer) { - await this.requestAction({ - componentIdx, - actionName: "submitAnswer", - }); - } - } + return this.autoSubmitManager.submitNow(); } requestComponentDoenetML(componentIdx, displayOnlyChildren) { @@ -13719,11 +12795,7 @@ export default class Core { } navigateToTarget(args) { - postMessage({ - messageType: "navigateToTarget", - coreId: this.coreId, - args, - }); + return this.navigationHandler.navigateToTarget(args); } } diff --git a/packages/doenetml-worker-javascript/src/DiagnosticsManager.ts b/packages/doenetml-worker-javascript/src/DiagnosticsManager.ts new file mode 100644 index 000000000..81d4787a7 --- /dev/null +++ b/packages/doenetml-worker-javascript/src/DiagnosticsManager.ts @@ -0,0 +1,182 @@ +type DiagnosticType = "error" | "warning" | "info" | "accessibility"; +type DiagnosticLevel = 1 | 2; + +type DiagnosticInput = { + type: DiagnosticType; + level?: DiagnosticLevel; + message: string; + position?: any; + sourceDoc?: number; +}; + +type DiagnosticRecord = { + type: DiagnosticType; + level?: DiagnosticLevel; + message: string; + position?: any; + sourceDoc?: number; +}; + +type AssertableDiagnostic = { + type: DiagnosticType; + level?: DiagnosticLevel; +}; + +/** + * Owns the diagnostics queue (errors, warnings, info, accessibility) for a Core + * instance. Core delegates to this manager for adding and reading diagnostics. + * + * Holds a back-reference to Core so `getSourceLocationForComponent` can walk + * the parent chain via `core._components`. + */ +export class DiagnosticsManager { + core: any; + diagnostics: DiagnosticRecord[]; + hasPendingDiagnostics: boolean; + + constructor({ + core, + preliminaryDiagnostics, + }: { + core: any; + preliminaryDiagnostics: DiagnosticInput[]; + }) { + this.core = core; + + this.diagnostics = preliminaryDiagnostics + // Note: we ignore preliminary errors, as we'll gather those from the dast when processing it. + .filter((diagnostic) => diagnostic.type !== "error") + .map((diagnostic) => { + this.assertDiagnosticIsValid(diagnostic); + + return { + type: diagnostic.type, + ...(diagnostic.type === "accessibility" + ? { level: diagnostic.level } + : {}), + message: diagnostic.message, + position: diagnostic.position, + sourceDoc: diagnostic.sourceDoc, + }; + }); + + this.hasPendingDiagnostics = true; + } + + /** + * Get pending diagnostics and reset the pending flag. + * Automatically trims the diagnostics array to prevent unbounded memory growth. + * + * @returns Object containing the current diagnostics array + * @note Diagnostics older than the 1000 most recent are discarded to manage memory + */ + getDiagnostics(): { diagnostics: DiagnosticRecord[] } { + // Keep only the last 1000 diagnostics to avoid unbounded memory growth. + // Once the limit is exceeded, older diagnostics are discarded. + // This ensures the codebase doesn't accumulate large numbers of stale messages. + const MAX_DIAGNOSTICS = 1000; + this.diagnostics = this.diagnostics.slice(-MAX_DIAGNOSTICS); + + this.hasPendingDiagnostics = false; + + return { diagnostics: this.diagnostics }; + } + + assertDiagnosticIsValid({ type, level }: AssertableDiagnostic): void { + if (!["error", "warning", "info", "accessibility"].includes(type)) { + throw Error("Invalid diagnostic type"); + } + + if (type === "accessibility") { + if (level === undefined) { + throw Error("Missing accessibility diagnostic level"); + } + + if (![1, 2].includes(level)) { + throw Error("Invalid accessibility diagnostic level"); + } + } + } + + /** + * Add a diagnostic record to `this.diagnostics`, deduplicating by + * type + message + source location. + * + * @returns `true` if a new entry was added, `false` if deduped. + */ + addDiagnostic({ + type, + level, + message, + position, + sourceDoc, + }: DiagnosticInput): boolean { + const sameLocation = (pointA: any, pointB: any) => + (pointA?.offset ?? undefined) === (pointB?.offset ?? undefined) && + (pointA?.line ?? undefined) === (pointB?.line ?? undefined) && + (pointA?.column ?? undefined) === (pointB?.column ?? undefined); + + const haveSamePosition = (warningPosition: any, newPosition: any) => { + if (warningPosition === undefined || newPosition === undefined) { + return warningPosition === newPosition; + } + + return ( + sameLocation(warningPosition.start, newPosition.start) && + sameLocation(warningPosition.end, newPosition.end) + ); + }; + + this.assertDiagnosticIsValid({ type, level }); + + const alreadyHaveDiagnostic = this.diagnostics.some( + (diagnostic) => + diagnostic.type === type && + (type === "accessibility" + ? diagnostic.level === level + : true) && + diagnostic.message === message && + diagnostic.sourceDoc === sourceDoc && + haveSamePosition(diagnostic.position, position), + ); + + if (alreadyHaveDiagnostic) { + return false; + } + + this.diagnostics.push({ + type, + ...(type === "accessibility" ? { level } : {}), + message, + position, + sourceDoc, + }); + + this.hasPendingDiagnostics = true; + return true; + } + + /** + * Find the nearest available source position/sourceDoc for a component, + * walking up ancestors when the component itself has no position. + */ + getSourceLocationForComponent(component: any): { + position: any; + sourceDoc: number | undefined; + } { + let position = component.position; + let sourceDoc = component.sourceDoc; + let comp = component; + + while (position === undefined) { + if (!(comp.parentIdx > 0)) { + break; + } + comp = this.core._components[comp.parentIdx]; + position = comp.position; + sourceDoc = comp.sourceDoc; + } + + return { position, sourceDoc }; + } +} diff --git a/packages/doenetml-worker-javascript/src/NavigationHandler.ts b/packages/doenetml-worker-javascript/src/NavigationHandler.ts new file mode 100644 index 000000000..1befbee1c --- /dev/null +++ b/packages/doenetml-worker-javascript/src/NavigationHandler.ts @@ -0,0 +1,66 @@ +/** + * Handles navigation actions: revealing closed sections in the ancestor chain + * before navigating to a target component, and posting `navigateToTarget` + * messages back to the host. + * + * Stateless — holds a back-reference to Core to read `_components`, + * dispatch `performAction`, and read `coreId`. + */ +export class NavigationHandler { + core: any; + + constructor({ core }: { core: any }) { + this.core = core; + } + + async handleNavigatingToComponent({ + componentIdx, + hash, + }: { + componentIdx: number; + hash: string; + }): Promise { + let component = this.core._components[componentIdx]; + if (!component) { + return; + } + let componentAndAncestors = [ + componentIdx, + ...component.ancestors.map((x: any) => x.componentIdx), + ]; + let openedParent = false; + for (let cIdx of componentAndAncestors) { + let comp = this.core._components[cIdx]; + if (comp.actions?.revealSection) { + let isOpen = await comp.stateValues.open; + + if (isOpen === false) { + await this.core.performAction({ + componentIdx: cIdx, + actionName: "revealSection", + }); + if (cIdx !== componentIdx) { + openedParent = true; + } + } + } + } + if (openedParent) { + // If just opened parent, then we couldn't have navigated to target yet + // as the target didn't exist in the DOM when the parent was closed. + // Navigate to the specified hash now. + postMessage({ + messageType: "navigateToHash", + args: { hash }, + }); + } + } + + navigateToTarget(args: any): void { + postMessage({ + messageType: "navigateToTarget", + coreId: this.core.coreId, + args, + }); + } +} diff --git a/packages/doenetml-worker-javascript/src/ResolverAdapter.ts b/packages/doenetml-worker-javascript/src/ResolverAdapter.ts new file mode 100644 index 000000000..6168d8e23 --- /dev/null +++ b/packages/doenetml-worker-javascript/src/ResolverAdapter.ts @@ -0,0 +1,459 @@ +import { assignDoenetMLRange } from "@doenet/utils"; +import { + addNodesToFlatFragment, + getEffectiveComponentIdx, +} from "./utils/resolver"; + +/** + * Adapter to the external (Rust) name resolver. Translates Core's + * component-tree concepts (components, replacements, composites) into the + * flat-fragment shape the resolver expects, and refreshes `core.rootNames` + * after each mutation. + * + * Stateless — the resolver lives outside this process. Holds a back-reference + * to Core to read `_components` / `componentInfoObjects`, invoke the resolver + * callbacks (`addNodesToResolver`, `deleteNodesFromResolver`, + * `calculateRootNames`), append diagnostics, and notify + * `dependencies.addBlockersFromChangedReplacements`. + */ +export class ResolverAdapter { + core: any; + + constructor({ core }: { core: any }) { + this.core = core; + } + + async addReplacementsToResolver({ + serializedReplacements, + component, + updateOldReplacementsStart, + updateOldReplacementsEnd, + blankStringReplacements, + }: { + serializedReplacements: any[]; + component: any; + updateOldReplacementsStart?: number; + updateOldReplacementsEnd?: number; + blankStringReplacements?: boolean[]; + }): Promise { + if (component.constructor.replacementsAlreadyInResolver) { + return; + } + + const { parentIdx, indexResolution } = + await this.determineParentAndIndexResolutionForResolver({ + component, + updateOldReplacementsStart, + updateOldReplacementsEnd, + blankStringReplacements, + }); + + // If `createComponentIdx` was specified, the one replacement is already in the resolver, + // so we just add its children and attribute components/references. + // Otherwise add all replacements. + const fragmentChildren: any[] = []; + let parentSourceSequence: any = null; + if (component.attributes.createComponentIdx != null) { + if (serializedReplacements[0]?.children) { + fragmentChildren.push(...serializedReplacements[0].children); + } + for (const attrName in serializedReplacements[0]?.attributes) { + const attribute = + serializedReplacements[0].attributes[attrName]; + if (attribute.type === "component") { + fragmentChildren.push(attribute.component); + } else if (attribute.type === "references") { + fragmentChildren.push(...attribute.references); + } + } + + // if the replacement that is the fragment parent has a source sequence, + // then add that as the `parentSourceSequence` of the flat fragment + let sourceSequence = + serializedReplacements[0]?.attributes["source:sequence"]; + if (sourceSequence) { + parentSourceSequence = { + type: "attribute", + name: "source:sequence", + parent: component.attributes.createComponentIdx.primitive + .number, + children: sourceSequence.children.filter( + (child: any) => typeof child === "string", + ), + sourceDoc: sourceSequence.sourceDoc, + }; + } + } else { + fragmentChildren.push(...serializedReplacements); + } + + // We add all the parent's descendants to the resolver + const flatFragment = { + children: fragmentChildren.map((child) => + typeof child === "string" + ? child + : getEffectiveComponentIdx(child), + ), + nodes: [], + parentIdx, + parentSourceSequence, + idxMap: {}, + }; + + addNodesToFlatFragment({ + flatFragment, + serializedComponents: fragmentChildren, + parentIdx, + }); + + if ( + (flatFragment.nodes.length > 0 || indexResolution !== "None") && + this.core.addNodesToResolver + ) { + this.core.addNodesToResolver(flatFragment, indexResolution); + + this.core.rootNames = this.core.calculateRootNames?.().names; + + let indexParent = + indexResolution.ReplaceAll?.parent ?? + indexResolution.ReplaceRange?.parent ?? + null; + + if ( + indexParent !== null && + indexParent !== component.componentIdx + ) { + const indexParentComposite = this.core._components[indexParent]; + + if (indexParentComposite) { + await this.core.dependencies.addBlockersFromChangedReplacements( + indexParentComposite, + ); + } + } + } + } + + async determineParentAndIndexResolutionForResolver({ + component, + updateOldReplacementsStart, + updateOldReplacementsEnd, + blankStringReplacements, + }: { + component: any; + updateOldReplacementsStart?: number; + updateOldReplacementsEnd?: number; + blankStringReplacements?: boolean[]; + }): Promise<{ parentIdx: number; indexResolution: any }> { + // If the composite was created as a child for a list, + // then the parent for resolving names is that list (the parent of the resolver). + // If `createComponentIdx` was specified, then that should be the parent for resolving names. + // Else, the composite should be the parent for resolving names. + + let update_start = updateOldReplacementsStart; + let update_end = updateOldReplacementsEnd; + + if ( + updateOldReplacementsStart !== undefined && + updateOldReplacementsEnd !== undefined + ) { + // We are replacing a range of replacement, but these include blank strings. + // Adjust the range to ignore blank strings + for (const [ + i, + isBlankString, + ] of (blankStringReplacements ?? []).entries()) { + if (i >= updateOldReplacementsEnd) { + break; + } + if (isBlankString) { + update_end!--; + if (i < updateOldReplacementsStart) { + update_start!--; + } + } + } + } + + let parentIdx: number; + + let indexResolution: any = "None"; + + if (component.doenetAttributes.forList) { + // Don't add index resolutions in this case, + // we're just adding to the children of the list, not the replacements of the list + parentIdx = component.parentIdx; + } else if (component.attributes.createComponentIdx?.primitive) { + // If `createComponentIdx` is set, then we have a copy component created from an `extend` attribute. + // That component is already in the resolver so will be the parent of the fragment added to the browser. + parentIdx = + component.attributes.createComponentIdx?.primitive.value; + + // If the component type of that parent, specified by `createComponentOfType`, is a composite, + // then it could have an index specified, so we add an index resolution + if ( + component.attributes.createComponentOfType?.primitive && + this.core.componentInfoObjects.isCompositeComponent({ + componentType: + component.attributes.createComponentOfType.primitive + .value, + includeNonStandard: true, + }) + ) { + indexResolution = { ReplaceAll: { parent: parentIdx } }; + + if (update_start !== undefined && update_end !== undefined) { + indexResolution = { + ReplaceRange: { + parent: parentIdx, + range: { start: update_start, end: update_end }, + }, + }; + } + } + } else if (component.componentType === "_copy") { + // If we have a copy that wasn't from an extend, then it was from a reference. + // Although references don't have names that can be + // Copy components are typically not part of the resolver structure and generally skipped. + // Since we don't allow direct authoring of copy components, + // they should occur only from references + + // determine if is a replacement of another type of composite + let copyComponent = component; + parentIdx = component.componentIdx; + + while (copyComponent.replacementOf) { + if (copyComponent.replacementOf.componentType === "_copy") { + copyComponent = copyComponent.replacementOf; + continue; + } else { + break; + } + } + + // now we have a copyComponent that is not a replacement of a copy + if (copyComponent.replacementOf) { + const indexParent = copyComponent.replacementOf; + + // determine where the replacement will end up being spliced in + + let start_idx: number | undefined; + let end_idx: number | undefined; + + async function calcStartEndIdx( + replacements: any[], + ): Promise { + let nonWithheldReplacements: any[] = []; + for (const repl of replacements) { + if ( + typeof repl === "string" || + !(await repl.stateValues + .isInactiveCompositeReplacement) + ) { + nonWithheldReplacements.push(repl); + } + } + + const nonBlankStringReplacements = + nonWithheldReplacements.filter( + (x) => typeof x !== "string" || x.trim() !== "", + ); + const replacementsWithoutExpandedCopies: any[] = []; + + let i = 0; + + for (const repl of nonBlankStringReplacements) { + if (repl.componentType == "_copy") { + if (!repl.isExpanded) { + if ( + repl.componentIdx === + copyComponent.componentIdx + ) { + start_idx = i; + end_idx = i + 1; + } + replacementsWithoutExpandedCopies.push(repl); + i++; + } else { + let replReplacements = repl.replacements; + if (repl.replacementsToWithhold) { + replReplacements = replReplacements.slice( + 0, + replReplacements.length - + repl.replacementsToWithhold, + ); + } + + const newReplacements = + await calcStartEndIdx(replReplacements); + const n = newReplacements.length; + + if ( + repl.componentIdx === + copyComponent.componentIdx + ) { + if ( + update_start !== undefined && + update_end !== undefined + ) { + start_idx = i + update_start; + end_idx = i + update_end; + } else { + start_idx = i; + end_idx = i + n; + } + } + + replacementsWithoutExpandedCopies.push( + ...newReplacements, + ); + i += n; + } + } else { + replacementsWithoutExpandedCopies.push(repl); + i++; + } + } + + return replacementsWithoutExpandedCopies; + } + + await calcStartEndIdx(indexParent.replacements); + + if (start_idx !== undefined && end_idx !== undefined) { + indexResolution = { + ReplaceRange: { + parent: indexParent.componentIdx, + range: { start: start_idx, end: end_idx }, + }, + }; + } else { + // if the copy was not found as a replacement of the composite, + // then it wasn't a top-level replacement and it doesn't affect the composite's index resolution + indexResolution = "None"; + } + } else { + parentIdx = copyComponent.componentIdx; + indexResolution = { ReplaceAll: { parent: parentIdx } }; + } + } else { + parentIdx = component.componentIdx; + + if ( + this.core.componentInfoObjects.isCompositeComponent({ + componentType: component.componentType, + includeNonStandard: true, + }) + ) { + if (update_start !== undefined && update_end !== undefined) { + indexResolution = { + ReplaceRange: { + parent: parentIdx, + range: { start: update_start, end: update_end }, + }, + }; + } else { + indexResolution = { ReplaceAll: { parent: parentIdx } }; + } + } + } + + return { parentIdx, indexResolution }; + } + + addComponentsToResolver(components: any[], parentIdx: number): void { + const flatFragment = { + children: components.map((child) => + typeof child === "string" + ? child + : getEffectiveComponentIdx(child), + ), + nodes: [], + parentIdx, + idxMap: {}, + }; + + addNodesToFlatFragment({ + flatFragment, + serializedComponents: components, + parentIdx, + }); + + if (this.core.addNodesToResolver) { + this.core.addNodesToResolver(flatFragment, "None"); + + this.core.rootNames = this.core.calculateRootNames?.().names; + } + } + + gatherDiagnosticsAndAssignDoenetMLRange({ + components, + diagnostics, + position, + sourceDoc, + overwriteDoenetMLRange = false, + }: { + components: any; + diagnostics: any[]; + position?: any; + sourceDoc?: number; + overwriteDoenetMLRange?: boolean; + }): void { + assignDoenetMLRange( + components, + position, + sourceDoc, + overwriteDoenetMLRange, + ); + assignDoenetMLRange(diagnostics, position, sourceDoc); + + // Add all diagnostics, preserving their existing type field + for (const diagnostic of diagnostics) { + this.core.addDiagnostic(diagnostic); + } + } + + removeComponentsFromResolver(componentsToRemove: any[]): void { + if (componentsToRemove.length === 0) { + return; + } + + const flatElements = componentsToRemove.map((comp) => { + let flatElement: any = { + type: "element", + name: comp.componentType, + parent: comp.parentIdx, + children: [], + attributes: [], + idx: comp.componentIdx, + }; + + if (comp.attributes.createComponentName && !comp.isExpanded) { + flatElement.attributes.push({ + type: "attribute", + name: "name", + parent: comp.parentIdx, + children: [ + comp.attributes.createComponentName.primitive.value, + ], + }); + } else if (comp.attributes.name) { + flatElement.attributes.push({ + type: "attribute", + name: "name", + parent: comp.parentIdx, + children: [comp.attributes.name.primitive.value], + }); + } + return flatElement; + }); + + if (this.core.deleteNodesFromResolver) { + this.core.deleteNodesFromResolver({ + nodes: flatElements, + }); + + this.core.rootNames = this.core.calculateRootNames?.().names; + } + } +} diff --git a/packages/doenetml-worker-javascript/src/StatePersistence.ts b/packages/doenetml-worker-javascript/src/StatePersistence.ts new file mode 100644 index 000000000..be30c4ee2 --- /dev/null +++ b/packages/doenetml-worker-javascript/src/StatePersistence.ts @@ -0,0 +1,149 @@ +import { serializedComponentsReplacer, data_format_version } from "@doenet/utils"; +import { set as idb_set } from "idb-keyval"; + +/** + * Owns the save-to-localStorage and save-to-database pipeline for a Core + * instance, including throttle timers and the debounced save scheduler. + * + * Holds a back-reference to Core to read `cumulativeStateVariableChanges`, + * `rendererState`, `flags`, `document`, the activity/doc/attempt IDs, and + * `coreInfoString` (set by Core during `generateDast`), and to invoke + * `reportScoreAndStateCallback`. + * + * This is purely the persistence I/O — the essential-value write engine + * that produces `cumulativeStateVariableChanges` is a separate concern + * (see `processNewStateVariableValues` in Core, slated for Phase 4). + */ +export class StatePersistence { + core: any; + saveStateToDBTimerId: ReturnType | null; + saveDocStateTimeoutID: ReturnType | null; + docStateToBeSavedToDatabase: any; + changesToBeSaved: boolean; + + constructor({ core }: { core: any }) { + this.core = core; + this.saveStateToDBTimerId = null; + this.saveDocStateTimeoutID = null; + this.docStateToBeSavedToDatabase = null; + this.changesToBeSaved = false; + } + + /** + * Schedule a debounced `saveState` after `delayMs` milliseconds, replacing + * any previously scheduled save. + */ + scheduleSave(delayMs: number): void { + if (this.saveDocStateTimeoutID !== null) { + clearTimeout(this.saveDocStateTimeoutID); + } + this.saveDocStateTimeoutID = setTimeout(() => { + this.saveState(); + }, delayMs); + } + + async saveImmediately(): Promise { + if (this.saveDocStateTimeoutID) { + // if in debounce to save doc to local storage + // then immediate save to local storage + // and override timeout to save to database + clearTimeout(this.saveDocStateTimeoutID); + await this.saveState(true); + } else { + // else override timeout to save any pending changes to database + await this.saveChangesToDatabase(true); + } + } + + async saveState( + overrideThrottle = false, + onSubmission = false, + ): Promise { + this.saveDocStateTimeoutID = null; + + const core = this.core; + + if (!core.flags.allowSaveState && !core.flags.allowLocalState) { + return; + } + + let coreStateString = JSON.stringify( + core.cumulativeStateVariableChanges, + serializedComponentsReplacer, + ); + let rendererStateString: string | null = null; + + if (core.flags.saveRendererState) { + rendererStateString = JSON.stringify( + core.rendererState, + serializedComponentsReplacer, + ); + } + + if (core.flags.allowLocalState) { + await idb_set( + `${core.activityId}|${core.docId}|${core.attemptNumber}|${core.cid}`, + { + data_format_version, + coreState: coreStateString, + rendererState: rendererStateString, + coreInfo: core.coreInfoString, + }, + ); + } + + if (!core.flags.allowSaveState) { + return; + } + + this.docStateToBeSavedToDatabase = { + cid: core.cid, + coreInfo: core.coreInfoString, + coreState: coreStateString, + rendererState: rendererStateString, + initializeCounters: core.initializeCounters, + docId: core.docId, + attemptNumber: core.attemptNumber, + activityId: core.activityId, + onSubmission, + }; + + // mark presence of changes + // so that next call to saveChangesToDatabase will save changes + this.changesToBeSaved = true; + + // if not currently in throttle, save changes to database + await this.saveChangesToDatabase(overrideThrottle); + } + + async saveChangesToDatabase(overrideThrottle = false): Promise { + // throttle save to database at 60 seconds + + if (!this.changesToBeSaved) { + return; + } + + if (this.saveStateToDBTimerId !== null) { + if (overrideThrottle) { + clearTimeout(this.saveStateToDBTimerId); + } else { + return; + } + } + + this.changesToBeSaved = false; + + // check for changes again after 60 seconds + this.saveStateToDBTimerId = setTimeout(() => { + this.saveStateToDBTimerId = null; + this.saveChangesToDatabase(); + }, 60000); + + this.core.reportScoreAndStateCallback({ + state: { ...this.docStateToBeSavedToDatabase }, + score: await this.core.document.stateValues.creditAchieved, + }); + + return; + } +} diff --git a/packages/doenetml-worker-javascript/src/StateVariableNameResolver.ts b/packages/doenetml-worker-javascript/src/StateVariableNameResolver.ts new file mode 100644 index 000000000..1ade343aa --- /dev/null +++ b/packages/doenetml-worker-javascript/src/StateVariableNameResolver.ts @@ -0,0 +1,275 @@ +// Pure string utilities for resolving state variable names: case-insensitive +// matching, public-only filtering, alias substitution, and array entry detection. +// +// Core wraps these so existing callers like +// `core.publicCaseInsensitiveAliasSubstitutions(args)` and the by-reference +// passes used in composite sugar functions keep working. + +export function findCaseInsensitiveMatches({ + stateVariables, + componentClass, + componentInfoObjects, +}: { + stateVariables: string[]; + componentClass: any; + componentInfoObjects: any; +}): string[] { + let stateVarInfo = + componentInfoObjects.stateVariableInfo[componentClass.componentType]; + + let newVariables: string[] = []; + + for (let stateVariable of stateVariables) { + let foundMatch = false; + + let lowerCaseVarName = stateVariable.toLowerCase(); + + for (let varName in stateVarInfo.stateVariableDescriptions) { + if (lowerCaseVarName === varName.toLowerCase()) { + foundMatch = true; + newVariables.push(varName); + break; + } + } + + if (foundMatch) { + continue; + } + + let isArraySize = false; + let lowerCaseNameMinusSize = lowerCaseVarName; + if (lowerCaseVarName.substring(0, 13) === "__array_size_") { + isArraySize = true; + lowerCaseNameMinusSize = lowerCaseVarName.substring(13); + } + + for (let aliasName in stateVarInfo.aliases) { + if (lowerCaseNameMinusSize === aliasName.toLowerCase()) { + // don't substitute alias here, just fix case + if (isArraySize) { + aliasName = "__array_size_" + aliasName; + } + newVariables.push(aliasName); + foundMatch = true; + break; + } + } + if (foundMatch) { + continue; + } + + let arrayEntryPrefixesLongestToShortest = Object.keys( + stateVarInfo.arrayEntryPrefixes, + ).sort((a, b) => b.length - a.length); + for (let prefix of arrayEntryPrefixesLongestToShortest) { + if ( + lowerCaseVarName.substring(0, prefix.length) === + prefix.toLowerCase() + ) { + // TODO: the varEnding is still a case-senstitive match + // Should we require that getArrayKeysFromVarName have + // a case-insensitive mode? + let arrayVariableName = + stateVarInfo.arrayEntryPrefixes[prefix].arrayVariableName; + let arrayStateVarDescription = + stateVarInfo.stateVariableDescriptions[arrayVariableName]; + let arrayKeys = + arrayStateVarDescription.getArrayKeysFromVarName({ + arrayEntryPrefix: prefix, + varEnding: stateVariable.substring(prefix.length), + numDimensions: arrayStateVarDescription.numDimensions, + }); + if (arrayKeys.length > 0) { + let newVarName = + prefix + lowerCaseVarName.substring(prefix.length); + foundMatch = true; + newVariables.push(newVarName); + break; + } + } + } + + if (foundMatch) { + continue; + } + + // no match, so don't alter + newVariables.push(stateVariable); + } + + return newVariables; +} + +export function matchPublicStateVariables({ + stateVariables, + componentClass, + componentInfoObjects, +}: { + stateVariables: string[]; + componentClass: any; + componentInfoObjects: any; +}): string[] { + let stateVarInfo = + componentInfoObjects.publicStateVariableInfo[ + componentClass.componentType + ]; + + let newVariables: string[] = []; + + for (let stateVariable of stateVariables) { + if (stateVariable in stateVarInfo.stateVariableDescriptions) { + // found public + newVariables.push(stateVariable); + continue; + } + + let varName = stateVariable; + + if (varName in stateVarInfo.aliases) { + varName = stateVarInfo.aliases[varName]; + + // check again to see if alias is public + if (varName in stateVarInfo.stateVariableDescriptions) { + // found public + newVariables.push(varName); + continue; + } + } + + let foundMatch = false; + + let arrayEntryPrefixesLongestToShortest = Object.keys( + stateVarInfo.arrayEntryPrefixes, + ).sort((a, b) => b.length - a.length); + for (let prefix of arrayEntryPrefixesLongestToShortest) { + if (varName.substring(0, prefix.length) === prefix) { + let arrayVariableName = + stateVarInfo.arrayEntryPrefixes[prefix].arrayVariableName; + let arrayStateVarDescription = + stateVarInfo.stateVariableDescriptions[arrayVariableName]; + let arrayKeys = + arrayStateVarDescription.getArrayKeysFromVarName({ + arrayEntryPrefix: prefix, + varEnding: varName.substring(prefix.length), + numDimensions: arrayStateVarDescription.numDimensions, + }); + if (arrayKeys.length > 0) { + foundMatch = true; + break; + } + } + } + + if (foundMatch) { + newVariables.push(stateVariable); + } else { + // no match, so make it a name that won't match + newVariables.push("__not_public_" + stateVariable); + } + } + + return newVariables; +} + +export function substituteAliases({ + stateVariables, + componentClass, + componentInfoObjects, +}: { + stateVariables: string[]; + componentClass: any; + componentInfoObjects: any; +}): string[] { + let newVariables: string[] = []; + + let stateVarInfo = + componentInfoObjects.stateVariableInfo[componentClass.componentType]; + + for (let stateVariable of stateVariables) { + let isArraySize = false; + if (stateVariable.substring(0, 13) === "__array_size_") { + isArraySize = true; + stateVariable = stateVariable.substring(13); + } + stateVariable = + stateVariable in stateVarInfo.aliases + ? stateVarInfo.aliases[stateVariable] + : stateVariable; + if (isArraySize) { + stateVariable = "__array_size_" + stateVariable; + } + newVariables.push(stateVariable); + } + + return newVariables; +} + +export function publicCaseInsensitiveAliasSubstitutions({ + stateVariables, + componentClass, + componentInfoObjects, +}: { + stateVariables: string[]; + componentClass: any; + componentInfoObjects: any; +}): string[] { + let mappedVarNames = findCaseInsensitiveMatches({ + stateVariables, + componentClass, + componentInfoObjects, + }); + + mappedVarNames = matchPublicStateVariables({ + stateVariables: mappedVarNames, + componentClass, + componentInfoObjects, + }); + + mappedVarNames = substituteAliases({ + stateVariables: mappedVarNames, + componentClass, + componentInfoObjects, + }); + + return mappedVarNames; +} + +export function checkIfArrayEntry({ + stateVariable, + component, +}: { + stateVariable: string; + component: any; +}): + | { + isArrayEntry: true; + arrayVariableName: string; + arrayEntryPrefix: string; + } + | { isArrayEntry: false } { + // check if stateVariable begins when an arrayEntry + for (let arrayEntryPrefix in component.arrayEntryPrefixes) { + if ( + stateVariable.substring(0, arrayEntryPrefix.length) === + arrayEntryPrefix + ) { + let arrayVariableName = + component.arrayEntryPrefixes[arrayEntryPrefix]; + let arrayStateVarObj = component.state[arrayVariableName]; + let arrayKeys = arrayStateVarObj.getArrayKeysFromVarName({ + arrayEntryPrefix, + varEnding: stateVariable.substring(arrayEntryPrefix.length), + numDimensions: arrayStateVarObj.numDimensions, + }); + if (arrayKeys.length > 0) { + return { + isArrayEntry: true, + arrayVariableName, + arrayEntryPrefix, + }; + } + } + } + + return { isArrayEntry: false }; +} diff --git a/packages/doenetml-worker-javascript/src/VisibilityTracker.ts b/packages/doenetml-worker-javascript/src/VisibilityTracker.ts new file mode 100644 index 000000000..76599b12c --- /dev/null +++ b/packages/doenetml-worker-javascript/src/VisibilityTracker.ts @@ -0,0 +1,186 @@ +type VisibilityInfo = { + componentsCurrentlyVisible: Record; + infoToSend: Record; + timeLastSent: Date; + saveDelay: number; + saveTimerId: ReturnType | null; + suspendDelay: number; + suspendTimerId: ReturnType | null; + suspended: boolean; + documentHasBeenVisible: boolean; +}; + +/** + * Tracks per-component visibility durations and emits aggregated + * `isVisible` events to the host. Owns timer state for the periodic + * "send" cycle and the auto-suspend after inactivity. + * + * Holds a back-reference to Core to push aggregated events onto + * `core.processQueue` and to call `core.onDocumentFirstVisible()` the + * first time the document becomes visible. + */ +export class VisibilityTracker { + core: any; + info: VisibilityInfo; + + constructor({ core }: { core: any }) { + this.core = core; + this.info = { + componentsCurrentlyVisible: {}, + infoToSend: {}, + timeLastSent: new Date(), + saveDelay: 60000, + saveTimerId: null, + suspendDelay: 3 * 60000, + suspendTimerId: null, + suspended: false, + documentHasBeenVisible: false, + }; + } + + processVisibilityChangedEvent(event: any): void { + let componentIdx = event.object.componentIdx; + let isVisible = event.result.isVisible; + + if (isVisible) { + if (!this.info.componentsCurrentlyVisible[componentIdx]) { + this.info.componentsCurrentlyVisible[componentIdx] = new Date(); + } + if (componentIdx === this.core.documentIdx) { + if (!this.info.documentHasBeenVisible) { + this.info.documentHasBeenVisible = true; + this.core.onDocumentFirstVisible(); + } + } + } else { + let begin = this.info.componentsCurrentlyVisible[componentIdx]; + if (begin) { + delete this.info.componentsCurrentlyVisible[componentIdx]; + + let timeInSeconds = + (new Date().getTime() - + Math.max( + begin.getTime(), + this.info.timeLastSent.getTime(), + )) / + 1000; + + if (this.info.infoToSend[componentIdx]) { + this.info.infoToSend[componentIdx] += timeInSeconds; + } else { + this.info.infoToSend[componentIdx] = timeInSeconds; + } + } + } + } + + sendVisibilityChangedEvents(): Promise | undefined { + let infoToSend: Record = { ...this.info.infoToSend }; + this.info.infoToSend = {}; + let timeLastSent = this.info.timeLastSent; + this.info.timeLastSent = new Date(); + let currentVisible: Record = { + ...this.info.componentsCurrentlyVisible, + }; + + for (const componentIdxStr in currentVisible) { + let timeInSeconds = + (this.info.timeLastSent.getTime() - + Math.max( + timeLastSent.getTime(), + currentVisible[componentIdxStr].getTime(), + )) / + 1000; + if (infoToSend[componentIdxStr]) { + infoToSend[componentIdxStr] += timeInSeconds; + } else { + infoToSend[componentIdxStr] = timeInSeconds; + } + } + + for (const componentIdxStr in infoToSend) { + infoToSend[componentIdxStr] = Math.round( + infoToSend[componentIdxStr], + ); + if (!infoToSend[componentIdxStr]) { + // delete if rounded down to zero + delete infoToSend[componentIdxStr]; + } + } + + let promise: Promise | undefined; + + if (Object.keys(infoToSend).length > 0) { + let event = { + object: { + componentIdx: this.core.documentIdx, + componentType: "document", + }, + verb: "isVisible", + result: infoToSend, + }; + + promise = new Promise((resolve, reject) => { + this.core.processQueue.push({ + type: "recordEvent", + event, + resolve, + reject, + }); + + if (!this.core.processing) { + this.core.processing = true; + this.core.executeProcesses(); + } + }); + } + + if (!this.info.suspended) { + if (this.info.saveTimerId !== null) { + clearTimeout(this.info.saveTimerId); + } + this.info.saveTimerId = setTimeout( + () => this.sendVisibilityChangedEvents(), + this.info.saveDelay, + ); + } + + return promise; + } + + async suspendVisibilityMeasuring(): Promise { + if (this.info.saveTimerId !== null) { + clearTimeout(this.info.saveTimerId); + } + if (this.info.suspendTimerId !== null) { + clearTimeout(this.info.suspendTimerId); + } + if (!this.info.suspended) { + this.info.suspended = true; + await this.sendVisibilityChangedEvents(); + } + } + + resumeVisibilityMeasuring(): void { + if (this.info.suspended) { + // restart visibility measuring + this.info.suspended = false; + this.info.timeLastSent = new Date(); + if (this.info.saveTimerId !== null) { + clearTimeout(this.info.saveTimerId); + } + this.info.saveTimerId = setTimeout( + () => this.sendVisibilityChangedEvents(), + this.info.saveDelay, + ); + } + + if (this.info.suspendTimerId !== null) { + clearTimeout(this.info.suspendTimerId); + } + this.info.suspendTimerId = setTimeout( + () => this.suspendVisibilityMeasuring(), + this.info.suspendDelay, + ); + } +} From 45050f627362797522754ee832c8e7593048e79e Mon Sep 17 00:00:00 2001 From: Duane Nykamp Date: Fri, 1 May 2026 14:15:01 -0500 Subject: [PATCH 2/8] docs: add CLAUDE.md with codebase guidance Co-Authored-By: Claude Haiku 4.5 --- CLAUDE.md | 174 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 CLAUDE.md diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 000000000..73815e200 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,174 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## High-Level Architecture + +DoenetML is a semantic markup language for building interactive web activities. The system has three layers: + +### Layer 1: Parsing (Input) +The **parser** (`packages/parser`) converts DoenetML XML into a **DAST** (Document Abstract Syntax Tree). It handles XML parsing, validation, and normalization. Key exports: `stringToLezer()`, `lezerToDast()`, `normalizeDocumentDast()`. + +### Layer 2: Computation (Worker) +The **worker** (`packages/doenetml-worker`) runs in a Web Worker and manages document state and computation. It combines: +- **JavaScript logic** (`packages/doenetml-worker-javascript`) for state updates and interactions +- **Rust/WASM logic** (`packages/doenetml-worker-rust/lib-js-wasm-binding`) The worker is slowly being transitioned to Rust. For the main `packages/doenetml` component, just small pieces of Rust (e.g., reference resolution) are currently invoked. + +Communication between main thread and worker uses structured messages. The worker is responsible for evaluating components, tracking dependencies (DAG), and managing variants. + +### Layer 3: UI (Viewer/Editor) +The **main component** (`packages/doenetml/src/doenetml.tsx`) exports two top-level React components: +- **`DoenetViewer`** — read-only rendering of DoenetML +- **`DoenetEditor`** — editor UI with live preview + +Both use Redux for state management (`packages/doenetml/src/state/`) and share a Web Worker instance. + +### Connected Packages +- **`@doenet/prefigure`** — backend for executing Python/computation-heavy activities +- **`@doenet/standalone`, `@doenet/doenetml-iframe`** — bundled variants of the main library for different hosting scenarios +- **`@doenet/codemirror`** — code editor integration +- **`@doenet/ui-components`** — reusable UI components (used across doenetml, prefigure, etc.) +- **`packages/vscode-extension`** — VS Code extension with LSP support (`packages/lsp`) + +## Monorepo Structure + +This is an npm workspace monorepo. Key points: +- All packages build via **Vite** and **Wireit** (a task orchestration tool that manages build dependencies) +- **Wireit** is configured in each `package.json`'s `wireit` field; it automatically rebuilds dependencies when inputs change +- Each package can be built/tested independently with `-w ` or `-w @scope/package-name` flags + +## Build & Development Commands + +### Daily Development + +```bash +# Start dev server (port 8012, builds doenetml and dependencies) +npm run dev + +# Build a single package (rebuilds dependencies automatically via wireit) +npm run build -w @doenet/doenetml + +# Build all packages (one-shot, no watch) +npm run build:all + +# Format code with Prettier (required before commits) +npm run prettier:format + +# Check formatting +npm run prettier:check +``` + +### Testing + +```bash +# Run all tests (Vitest only, very slow) +npm run test + +# Run Vitest only (all packages except worker) +npm run test:all-no-worker-js -- run + +# Run targeted Vitest (e.g., prefigure package) +npm run test -w @doenet/prefigure -- --run test/index-api.test.ts + +# Run Cypress e2e tests in groups (recommended) +npm run test:e2e-group1 +npm run test:e2e-group2 +npm run test:e2e-group3 +npm run test:e2e-group4 +npm run test:e2e-group5 +npm run test:codemirror-cypress + +# Run a single Cypress spec (fast-fail mode) +npm run test-cypress-fast-fail -w @doenet/test-cypress -- --config specPattern=cypress/e2e/tagSpecific/choiceinput.cy.js +``` + +**Important**: After code changes affecting Cypress, rebuild `@doenet/test-cypress` before running Cypress tests. See `TEST_RUN_INSTRUCTIONS_FOR_AGENTS.md` for the full runbook. + +## Coding Conventions + +From `AGENTS.md`; treat these as requirements: + +- **No `private` class fields.** Use an underscore prefix for internal members (`_field`). +- **No fire-and-forget promises.** Always attach `.catch()` to intentionally unawaited Promises, or use `async`/`await`. +- **Prefer function declarations over function-valued variables** (`function foo() {}` over `const foo = ()`), unless reassignment is needed. +- **Prefer `async`/`await`** over `.then()` chains. +- **Format with Prettier** before committing: `npm run prettier:format` + +## Files to Avoid Committing + +From `AGENTS.md`: + +- `packages/doenetml/dev/testCode.doenet` — local development file +- `packages/doenetml/dev/main.tsx` — local development file +- Untracked `*.md` files in the repository root — planning/notes files + +If you edit these during development, they will show as modified but should not be staged. + +## Changesets & Publishing + +The repo uses Changesets for version management. Configuration is in `.changeset/config.json`. + +### Fixed Group (synchronized versioning) +Six packages version together: +- `@doenet/doenetml`, `@doenet/standalone`, `@doenet/doenetml-iframe` +- `@doenet/v06-to-v07`, `@doenet/vscode-extension`, `doenet-vscode-extension` + +### Independent Versioning +- `@doenet/prefigure` versions independently + +**Rule**: When creating a changeset for a user-facing change, include **all packages** where the change is visible to end users, not just where the implementation lives. + +Example: A change to `packages/doenetml/src/Viewer` affects `@doenet/doenetml`, `@doenet/standalone`, `@doenet/doenetml-iframe`, and downstream variants. Include all in the changeset. + +## GitHub & Remotes + +The repository may use a personal fork as `origin` with the canonical `Doenet/DoenetML` as `upstream`. + +- **Always base PRs on `upstream/main`**, not `origin/main` +- Push your branch to your fork (`origin`), then create the PR targeting `Doenet/DoenetML:main` +- Use the GitHub CLI: `gh pr create --repo Doenet/DoenetML --base main --head :` + +## Key State & Data Flow + +### Redux Store Structure +Located in `packages/doenetml/src/state/`: +- **`main` slice** — document state, component data, update queue +- **`keyboard` slice** — virtual keyboard focus tracking + +Components dispatch actions to update UI state; the worker listens for changes and updates document computation. + +### Worker Communication +The worker receives serialized updates and returns rendered component states. Redux selectors provide derived state to UI components. + +## Testing Strategy + +- **Vitest** for unit tests, component logic, and utility functions (files: `*.test.ts`, `*.test.tsx`) +- **Cypress** for e2e tests, user interactions, and full rendering (files: `cypress/e2e/*.cy.js`) +- Tests are grouped; run by group number to parallelize CI + +## Common Tasks + +### Add a new component type +1. Implement the **worker logic** in `packages/doenetml-worker-javascript` +2. Implement the **UI renderer** in `packages/doenetml/src/Viewer/renderers` or similar +3. Build the schema +4. Add **tests** in both Vitest and Cypress +5. Add a **changeset** if user-facing + +### Debug a rendering issue +1. Start `npm run dev` and inspect the browser console +2. Use Redux DevTools to inspect state changes +3. Run `npm run test -w @doenet/test-cypress` to verify e2e tests still pass + +### Run docs locally +```bash +npm run docs +``` + +Builds prerequisites and serves docs (Nextra-based) at `http://localhost:3000`. + +## Performance & Notes + +- The worker runs heavy computations off the main thread; avoid blocking UI updates +- Large documents or deeply nested variants can cause lag; consider profiling with DevTools +- Rust/WASM changes require a full rebuild of `packages/doenetml-worker-rust`; Wireit handles this but can be slow the first time From 36d6d30b54ad1c35aee4cfeee96499bd3e599b6d Mon Sep 17 00:00:00 2001 From: Duane Nykamp Date: Fri, 1 May 2026 14:28:52 -0500 Subject: [PATCH 3/8] prettier --- packages/doenetml-worker-javascript/src/ResolverAdapter.ts | 7 +++---- .../doenetml-worker-javascript/src/StatePersistence.ts | 5 ++++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/doenetml-worker-javascript/src/ResolverAdapter.ts b/packages/doenetml-worker-javascript/src/ResolverAdapter.ts index 6168d8e23..7dbdfecb9 100644 --- a/packages/doenetml-worker-javascript/src/ResolverAdapter.ts +++ b/packages/doenetml-worker-javascript/src/ResolverAdapter.ts @@ -159,10 +159,9 @@ export class ResolverAdapter { ) { // We are replacing a range of replacement, but these include blank strings. // Adjust the range to ignore blank strings - for (const [ - i, - isBlankString, - ] of (blankStringReplacements ?? []).entries()) { + for (const [i, isBlankString] of ( + blankStringReplacements ?? [] + ).entries()) { if (i >= updateOldReplacementsEnd) { break; } diff --git a/packages/doenetml-worker-javascript/src/StatePersistence.ts b/packages/doenetml-worker-javascript/src/StatePersistence.ts index be30c4ee2..3bb13daf3 100644 --- a/packages/doenetml-worker-javascript/src/StatePersistence.ts +++ b/packages/doenetml-worker-javascript/src/StatePersistence.ts @@ -1,4 +1,7 @@ -import { serializedComponentsReplacer, data_format_version } from "@doenet/utils"; +import { + serializedComponentsReplacer, + data_format_version, +} from "@doenet/utils"; import { set as idb_set } from "idb-keyval"; /** From aed7910ca649d85028cbae1accd37e9e37aa6c9d Mon Sep 17 00:00:00 2001 From: Duane Nykamp Date: Sat, 2 May 2026 23:07:57 -0500 Subject: [PATCH 4/8] refactor(worker-javascript): address Phase 1 PR review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address 9 Copilot review comments on PR #1036. All flagged code (the .primitive.number bug, six fire-and-forget Promises, the typo, four TS errors) pre-existed in Core.js and was either copied verbatim or exposed when the JS was converted to TS during extraction. - ResolverAdapter: fix latent createComponentIdx.primitive.number access (should be .value, with a primitive.type === "number" guard) — same pattern as utils/resolver.ts:220-224 and utils/componentIndices.ts:725. - ResolverAdapter: add type: "flatFragment" and parentSourceSequence fields to two FlatFragment literals; tighten gatherDiagnosticsAnd- AssignDoenetMLRange signature so position/sourceDoc match assign- DoenetMLRange's required arguments. - Add reportTimerError helper and attach .catch() handlers to six fire-and-forget Promises in setTimeout callbacks across StatePersistence (2), AutoSubmitManager (1), and VisibilityTracker (3), per AGENTS.md convention. Also clear submitAnswersTimeout when it fires. - Fix typo "case-senstitive" -> "case-sensitive" in StateVariableNameResolver. Documentation: merge CLAUDE.md into AGENTS.md as the single source of truth (project conventions, architecture, build commands, testing, common tasks). Reduce CLAUDE.md to a one-line pointer so Claude Code auto-context still finds the project guide while eliminating drift. Co-Authored-By: Claude Opus 4.7 (1M context) --- AGENTS.md | 195 +++++++++++++++--- CLAUDE.md | 173 +--------------- .../src/AutoSubmitManager.ts | 5 +- .../src/ResolverAdapter.ts | 22 +- .../src/StatePersistence.ts | 7 +- .../src/StateVariableNameResolver.ts | 2 +- .../src/VisibilityTracker.ts | 29 +-- .../src/utils/timerErrors.ts | 12 ++ 8 files changed, 219 insertions(+), 226 deletions(-) create mode 100644 packages/doenetml-worker-javascript/src/utils/timerErrors.ts diff --git a/AGENTS.md b/AGENTS.md index 50f3cae1f..6b6c21567 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,51 +1,184 @@ # AGENTS.md -## Testing +Canonical guide for agents (and humans) working in this repository. Covers architecture, build commands, testing, coding conventions, commit hygiene, PR creation, and changesets. + +## High-Level Architecture + +DoenetML is a semantic markup language for building interactive web activities. The system has three layers: + +### Layer 1: Parsing (Input) +The **parser** (`packages/parser`) converts DoenetML XML into a **DAST** (Document Abstract Syntax Tree). It handles XML parsing, validation, and normalization. Key exports: `stringToLezer()`, `lezerToDast()`, `normalizeDocumentDast()`. + +### Layer 2: Computation (Worker) +The **worker** (`packages/doenetml-worker`) runs in a Web Worker and manages document state and computation. It combines: +- **JavaScript logic** (`packages/doenetml-worker-javascript`) for state updates and interactions +- **Rust/WASM logic** (`packages/doenetml-worker-rust/lib-js-wasm-binding`). The worker is slowly being transitioned to Rust. For the main `packages/doenetml` component, just small pieces of Rust (e.g., reference resolution) are currently invoked. + +Communication between main thread and worker uses structured messages. The worker is responsible for evaluating components, tracking dependencies (DAG), and managing variants. + +### Layer 3: UI (Viewer/Editor) +The **main component** (`packages/doenetml/src/doenetml.tsx`) exports two top-level React components: +- **`DoenetViewer`** — read-only rendering of DoenetML +- **`DoenetEditor`** — editor UI with live preview + +Both use Redux for state management (`packages/doenetml/src/state/`) and share a Web Worker instance. + +### Connected Packages +- **`@doenet/prefigure`** — backend for executing Python/computation-heavy activities +- **`@doenet/standalone`, `@doenet/doenetml-iframe`** — bundled variants of the main library for different hosting scenarios +- **`@doenet/codemirror`** — code editor integration +- **`@doenet/ui-components`** — reusable UI components (used across doenetml, prefigure, etc.) +- **`packages/vscode-extension`** — VS Code extension with LSP support (`packages/lsp`) + +## Monorepo Structure + +This is an npm workspace monorepo. Key points: +- All packages build via **Vite** and **Wireit** (a task orchestration tool that manages build dependencies) +- **Wireit** is configured in each `package.json`'s `wireit` field; it automatically rebuilds dependencies when inputs change +- Each package can be built/tested independently with `-w ` or `-w @scope/package-name` flags + +## Build & Development Commands -Agents working in this repository should read [TEST_RUN_INSTRUCTIONS_FOR_AGENTS.md](TEST_RUN_INSTRUCTIONS_FOR_AGENTS.md) before running tests. +### Daily Development +```bash +# Start dev server (port 8012, builds doenetml and dependencies) +npm run dev + +# Build a single package (rebuilds dependencies automatically via wireit) +npm run build -w @doenet/doenetml + +# Build all packages (one-shot, no watch) +npm run build:all + +# Format code with Prettier (required before commits) +npm run prettier:format + +# Check formatting +npm run prettier:check +``` + +### Run docs locally + +```bash +npm run docs +``` + +Builds prerequisites and serves docs (Nextra-based) at `http://localhost:3000`. + +## Testing + +Read [TEST_RUN_INSTRUCTIONS_FOR_AGENTS.md](TEST_RUN_INSTRUCTIONS_FOR_AGENTS.md) before running tests. Highlights: - For `@doenet/test-cypress`, rebuild before Cypress runs after code changes. - Follow the required sequence: `build -> preview -> cypress run`. - Do not rely on an old build or an already-running preview server after source edits. - The runbook includes non-interactive test commands, Cypress preview-server workflow, fail-fast Cypress commands, and stale-asset troubleshooting. -## Coding Conventions for Agents +### Test Tooling + +- **Vitest** for unit tests, component logic, and utility functions (files: `*.test.ts`, `*.test.tsx`) +- **Cypress** for e2e tests, user interactions, and full rendering (files: `cypress/e2e/*.cy.js`) +- Tests are grouped; run by group number to parallelize CI -- Avoid `private` class fields and methods; use an underscore prefix for internal members. -- Avoid fire-and-forget calls via `void`; if a Promise is intentionally not awaited, attach an explicit `.catch(...)` handler. -- Prefer function declarations over function-valued variables, unless reassignment or dynamic replacement is required. -- Prefer `async`/`await` over old-style Promise chains (`.then(...)` / `.catch(...)`) when writing asynchronous code. +### Common Test Commands -## Commit Hygiene Requirements +```bash +# Run all tests (Vitest only, very slow) +npm run test -- Format changed files with Prettier before committing. -- Ignore changes to `testCode.doenet` files. Do not commit them. -- Ignore changes to `packages/doenetml/dev/main.tsx`. Do not commit them. -- Ignore changes to any untracked `*.md` files in the repository root. Do not commit them. +# Run Vitest only (all packages except worker) +npm run test:all-no-worker-js -- run + +# Run targeted Vitest (e.g., prefigure package) +npm run test -w @doenet/prefigure -- --run test/index-api.test.ts + +# Run Cypress e2e tests in groups (recommended) +npm run test:e2e-group1 +npm run test:e2e-group2 +npm run test:e2e-group3 +npm run test:e2e-group4 +npm run test:e2e-group5 +npm run test:codemirror-cypress + +# Run a single Cypress spec (fast-fail mode) +npm run test-cypress-fast-fail -w @doenet/test-cypress -- --config specPattern=cypress/e2e/tagSpecific/choiceinput.cy.js +``` + +## Coding Conventions + +- **No `private` class fields or methods.** Use an underscore prefix for internal members (`_field`). +- **No fire-and-forget promises.** Always attach an explicit `.catch(...)` handler to intentionally unawaited Promises, or use `async`/`await`. +- **Prefer function declarations** over function-valued variables (`function foo() {}` over `const foo = () => {}`), unless reassignment or dynamic replacement is required. +- **Prefer `async`/`await`** over `.then(...)` / `.catch(...)` chains. + +## Commit Hygiene + +- Format changed files with Prettier before committing: `npm run prettier:format` +- Files that should never be staged or committed (local development / planning notes): + - `packages/doenetml/dev/testCode.doenet` + - `packages/doenetml/dev/main.tsx` + - Untracked `*.md` files in the repository root + +If you edit these during development they will show as modified, but should not be staged. ## PR Creation -- This checkout may use a personal fork as `origin` and the canonical repository as `upstream`. -- Do not assume `origin/main` matches the PR base. Base pull requests on `upstream/main`. -- Push the branch to your fork, then create the PR in `Doenet/DoenetML` with your fork branch as the head. -- **Preferred method: Use GitHub CLI (`gh`).** The `mcp_gitkraken_pull_request_create` tool requires authentication that may not be available. +This checkout may use a personal fork as `origin` and the canonical `Doenet/DoenetML` as `upstream`. + +- **Always base PRs on `upstream/main`**, not `origin/main`. +- Push your branch to your fork (`origin`), then create the PR targeting `Doenet/DoenetML:main`. +- **Preferred method: GitHub CLI (`gh`).** The `mcp_gitkraken_pull_request_create` tool requires authentication that may not be available. - Command format: `gh pr create --repo Doenet/DoenetML --base main --head :`. Replace `` with your GitHub username (e.g., `dqnykamp:my-branch`). -- Before pushing, run `prettier` to format modified files and ensure formatting compliance. -- Before creating the PR, confirm that only intended files are staged and committed, since this repository often has unrelated local work in the tree. +- Before pushing, run `npm run prettier:format` on modified files. +- Before creating the PR, confirm only intended files are staged — this repository often has unrelated local work in the tree. - After creating the PR, verify the branch has been pushed to `origin` and the PR links to the correct target branch (`Doenet/DoenetML:main`). ## Changesets -- Changesets are configured in `.changeset/config.json`. -- The repo currently uses one `fixed` group containing six packages: - - `@doenet/doenetml` - - `@doenet/standalone` - - `@doenet/doenetml-iframe` - - `@doenet/v06-to-v07` - - `@doenet/vscode-extension` - - `doenet-vscode-extension` -- Additionally, `@doenet/prefigure` is published but with an independent version (not part of the fixed group). -- User-facing changes in `@doenet/doenetml` are also apparent in `@doenet/standalone`, `@doenet/doenetml-iframe`, `@doenet/vscode-extension`, and `doenet-vscode-extension`. -- User-facing changes in `@doenet/standalone` are also apparent in `@doenet/doenetml-iframe`. -- When writing a changeset, include the published package or packages where the user-facing change is apparent, not just the package where the implementation lives. -- The fixed-group configuration coordinates versioning across all six fixed-group packages. \ No newline at end of file +The repo uses Changesets for version management. Configuration is in `.changeset/config.json`. + +### Fixed Group (synchronized versioning) +Six packages version together: +- `@doenet/doenetml`, `@doenet/standalone`, `@doenet/doenetml-iframe` +- `@doenet/v06-to-v07`, `@doenet/vscode-extension`, `doenet-vscode-extension` + +### Independent Versioning +- `@doenet/prefigure` versions independently + +**Rule**: When creating a changeset for a user-facing change, include **all packages** where the change is visible to end users, not just where the implementation lives. + +Example: A change to `packages/doenetml/src/Viewer` affects `@doenet/doenetml`, `@doenet/standalone`, `@doenet/doenetml-iframe`, and downstream variants. Include all in the changeset. + +User-facing changes in `@doenet/standalone` are also apparent in `@doenet/doenetml-iframe`. + +## Key State & Data Flow + +### Redux Store Structure +Located in `packages/doenetml/src/state/`: +- **`main` slice** — document state, component data, update queue +- **`keyboard` slice** — virtual keyboard focus tracking + +Components dispatch actions to update UI state; the worker listens for changes and updates document computation. + +### Worker Communication +The worker receives serialized updates and returns rendered component states. Redux selectors provide derived state to UI components. + +## Common Tasks + +### Add a new component type +1. Implement the **worker logic** in `packages/doenetml-worker-javascript` +2. Implement the **UI renderer** in `packages/doenetml/src/Viewer/renderers` or similar +3. Build the schema +4. Add **tests** in both Vitest and Cypress +5. Add a **changeset** if user-facing + +### Debug a rendering issue +1. Start `npm run dev` and inspect the browser console +2. Use Redux DevTools to inspect state changes +3. Run `npm run test -w @doenet/test-cypress` to verify e2e tests still pass + +## Performance & Notes + +- The worker runs heavy computations off the main thread; avoid blocking UI updates +- Large documents or deeply nested variants can cause lag; consider profiling with DevTools +- Rust/WASM changes require a full rebuild of `packages/doenetml-worker-rust`; Wireit handles this but can be slow the first time diff --git a/CLAUDE.md b/CLAUDE.md index 73815e200..95161aa18 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,174 +1,3 @@ # CLAUDE.md -This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. - -## High-Level Architecture - -DoenetML is a semantic markup language for building interactive web activities. The system has three layers: - -### Layer 1: Parsing (Input) -The **parser** (`packages/parser`) converts DoenetML XML into a **DAST** (Document Abstract Syntax Tree). It handles XML parsing, validation, and normalization. Key exports: `stringToLezer()`, `lezerToDast()`, `normalizeDocumentDast()`. - -### Layer 2: Computation (Worker) -The **worker** (`packages/doenetml-worker`) runs in a Web Worker and manages document state and computation. It combines: -- **JavaScript logic** (`packages/doenetml-worker-javascript`) for state updates and interactions -- **Rust/WASM logic** (`packages/doenetml-worker-rust/lib-js-wasm-binding`) The worker is slowly being transitioned to Rust. For the main `packages/doenetml` component, just small pieces of Rust (e.g., reference resolution) are currently invoked. - -Communication between main thread and worker uses structured messages. The worker is responsible for evaluating components, tracking dependencies (DAG), and managing variants. - -### Layer 3: UI (Viewer/Editor) -The **main component** (`packages/doenetml/src/doenetml.tsx`) exports two top-level React components: -- **`DoenetViewer`** — read-only rendering of DoenetML -- **`DoenetEditor`** — editor UI with live preview - -Both use Redux for state management (`packages/doenetml/src/state/`) and share a Web Worker instance. - -### Connected Packages -- **`@doenet/prefigure`** — backend for executing Python/computation-heavy activities -- **`@doenet/standalone`, `@doenet/doenetml-iframe`** — bundled variants of the main library for different hosting scenarios -- **`@doenet/codemirror`** — code editor integration -- **`@doenet/ui-components`** — reusable UI components (used across doenetml, prefigure, etc.) -- **`packages/vscode-extension`** — VS Code extension with LSP support (`packages/lsp`) - -## Monorepo Structure - -This is an npm workspace monorepo. Key points: -- All packages build via **Vite** and **Wireit** (a task orchestration tool that manages build dependencies) -- **Wireit** is configured in each `package.json`'s `wireit` field; it automatically rebuilds dependencies when inputs change -- Each package can be built/tested independently with `-w ` or `-w @scope/package-name` flags - -## Build & Development Commands - -### Daily Development - -```bash -# Start dev server (port 8012, builds doenetml and dependencies) -npm run dev - -# Build a single package (rebuilds dependencies automatically via wireit) -npm run build -w @doenet/doenetml - -# Build all packages (one-shot, no watch) -npm run build:all - -# Format code with Prettier (required before commits) -npm run prettier:format - -# Check formatting -npm run prettier:check -``` - -### Testing - -```bash -# Run all tests (Vitest only, very slow) -npm run test - -# Run Vitest only (all packages except worker) -npm run test:all-no-worker-js -- run - -# Run targeted Vitest (e.g., prefigure package) -npm run test -w @doenet/prefigure -- --run test/index-api.test.ts - -# Run Cypress e2e tests in groups (recommended) -npm run test:e2e-group1 -npm run test:e2e-group2 -npm run test:e2e-group3 -npm run test:e2e-group4 -npm run test:e2e-group5 -npm run test:codemirror-cypress - -# Run a single Cypress spec (fast-fail mode) -npm run test-cypress-fast-fail -w @doenet/test-cypress -- --config specPattern=cypress/e2e/tagSpecific/choiceinput.cy.js -``` - -**Important**: After code changes affecting Cypress, rebuild `@doenet/test-cypress` before running Cypress tests. See `TEST_RUN_INSTRUCTIONS_FOR_AGENTS.md` for the full runbook. - -## Coding Conventions - -From `AGENTS.md`; treat these as requirements: - -- **No `private` class fields.** Use an underscore prefix for internal members (`_field`). -- **No fire-and-forget promises.** Always attach `.catch()` to intentionally unawaited Promises, or use `async`/`await`. -- **Prefer function declarations over function-valued variables** (`function foo() {}` over `const foo = ()`), unless reassignment is needed. -- **Prefer `async`/`await`** over `.then()` chains. -- **Format with Prettier** before committing: `npm run prettier:format` - -## Files to Avoid Committing - -From `AGENTS.md`: - -- `packages/doenetml/dev/testCode.doenet` — local development file -- `packages/doenetml/dev/main.tsx` — local development file -- Untracked `*.md` files in the repository root — planning/notes files - -If you edit these during development, they will show as modified but should not be staged. - -## Changesets & Publishing - -The repo uses Changesets for version management. Configuration is in `.changeset/config.json`. - -### Fixed Group (synchronized versioning) -Six packages version together: -- `@doenet/doenetml`, `@doenet/standalone`, `@doenet/doenetml-iframe` -- `@doenet/v06-to-v07`, `@doenet/vscode-extension`, `doenet-vscode-extension` - -### Independent Versioning -- `@doenet/prefigure` versions independently - -**Rule**: When creating a changeset for a user-facing change, include **all packages** where the change is visible to end users, not just where the implementation lives. - -Example: A change to `packages/doenetml/src/Viewer` affects `@doenet/doenetml`, `@doenet/standalone`, `@doenet/doenetml-iframe`, and downstream variants. Include all in the changeset. - -## GitHub & Remotes - -The repository may use a personal fork as `origin` with the canonical `Doenet/DoenetML` as `upstream`. - -- **Always base PRs on `upstream/main`**, not `origin/main` -- Push your branch to your fork (`origin`), then create the PR targeting `Doenet/DoenetML:main` -- Use the GitHub CLI: `gh pr create --repo Doenet/DoenetML --base main --head :` - -## Key State & Data Flow - -### Redux Store Structure -Located in `packages/doenetml/src/state/`: -- **`main` slice** — document state, component data, update queue -- **`keyboard` slice** — virtual keyboard focus tracking - -Components dispatch actions to update UI state; the worker listens for changes and updates document computation. - -### Worker Communication -The worker receives serialized updates and returns rendered component states. Redux selectors provide derived state to UI components. - -## Testing Strategy - -- **Vitest** for unit tests, component logic, and utility functions (files: `*.test.ts`, `*.test.tsx`) -- **Cypress** for e2e tests, user interactions, and full rendering (files: `cypress/e2e/*.cy.js`) -- Tests are grouped; run by group number to parallelize CI - -## Common Tasks - -### Add a new component type -1. Implement the **worker logic** in `packages/doenetml-worker-javascript` -2. Implement the **UI renderer** in `packages/doenetml/src/Viewer/renderers` or similar -3. Build the schema -4. Add **tests** in both Vitest and Cypress -5. Add a **changeset** if user-facing - -### Debug a rendering issue -1. Start `npm run dev` and inspect the browser console -2. Use Redux DevTools to inspect state changes -3. Run `npm run test -w @doenet/test-cypress` to verify e2e tests still pass - -### Run docs locally -```bash -npm run docs -``` - -Builds prerequisites and serves docs (Nextra-based) at `http://localhost:3000`. - -## Performance & Notes - -- The worker runs heavy computations off the main thread; avoid blocking UI updates -- Large documents or deeply nested variants can cause lag; consider profiling with DevTools -- Rust/WASM changes require a full rebuild of `packages/doenetml-worker-rust`; Wireit handles this but can be slow the first time +See [AGENTS.md](AGENTS.md) for project conventions, architecture, build commands, and agent guidelines. diff --git a/packages/doenetml-worker-javascript/src/AutoSubmitManager.ts b/packages/doenetml-worker-javascript/src/AutoSubmitManager.ts index e2de4e565..69a132d42 100644 --- a/packages/doenetml-worker-javascript/src/AutoSubmitManager.ts +++ b/packages/doenetml-worker-javascript/src/AutoSubmitManager.ts @@ -1,3 +1,5 @@ +import { reportTimerError } from "./utils/timerErrors"; + /** * Owns the debounced auto-submit-answer queue. When state changes record an * answer that should be submitted, the manager batches them and dispatches @@ -27,7 +29,8 @@ export class AutoSubmitManager { //Debounce the submit answers this.submitAnswersTimeout = setTimeout(() => { - this.submitNow(); + this.submitAnswersTimeout = null; + this.submitNow().catch(reportTimerError("auto-submit answers")); }, 1000); } diff --git a/packages/doenetml-worker-javascript/src/ResolverAdapter.ts b/packages/doenetml-worker-javascript/src/ResolverAdapter.ts index 7dbdfecb9..abac13326 100644 --- a/packages/doenetml-worker-javascript/src/ResolverAdapter.ts +++ b/packages/doenetml-worker-javascript/src/ResolverAdapter.ts @@ -1,4 +1,5 @@ import { assignDoenetMLRange } from "@doenet/utils"; +import { FlatFragment } from "@doenet/doenetml-worker"; import { addNodesToFlatFragment, getEffectiveComponentIdx, @@ -71,12 +72,16 @@ export class ResolverAdapter { // then add that as the `parentSourceSequence` of the flat fragment let sourceSequence = serializedReplacements[0]?.attributes["source:sequence"]; - if (sourceSequence) { + const createComponentIdxPrimitive = + component.attributes.createComponentIdx?.primitive; + if ( + sourceSequence && + createComponentIdxPrimitive?.type === "number" + ) { parentSourceSequence = { type: "attribute", name: "source:sequence", - parent: component.attributes.createComponentIdx.primitive - .number, + parent: createComponentIdxPrimitive.value, children: sourceSequence.children.filter( (child: any) => typeof child === "string", ), @@ -88,7 +93,8 @@ export class ResolverAdapter { } // We add all the parent's descendants to the resolver - const flatFragment = { + const flatFragment: FlatFragment = { + type: "flatFragment", children: fragmentChildren.map((child) => typeof child === "string" ? child @@ -361,7 +367,8 @@ export class ResolverAdapter { } addComponentsToResolver(components: any[], parentIdx: number): void { - const flatFragment = { + const flatFragment: FlatFragment = { + type: "flatFragment", children: components.map((child) => typeof child === "string" ? child @@ -369,6 +376,7 @@ export class ResolverAdapter { ), nodes: [], parentIdx, + parentSourceSequence: null, idxMap: {}, }; @@ -394,8 +402,8 @@ export class ResolverAdapter { }: { components: any; diagnostics: any[]; - position?: any; - sourceDoc?: number; + position: any; + sourceDoc: number; overwriteDoenetMLRange?: boolean; }): void { assignDoenetMLRange( diff --git a/packages/doenetml-worker-javascript/src/StatePersistence.ts b/packages/doenetml-worker-javascript/src/StatePersistence.ts index 3bb13daf3..8cecd65dd 100644 --- a/packages/doenetml-worker-javascript/src/StatePersistence.ts +++ b/packages/doenetml-worker-javascript/src/StatePersistence.ts @@ -3,6 +3,7 @@ import { data_format_version, } from "@doenet/utils"; import { set as idb_set } from "idb-keyval"; +import { reportTimerError } from "./utils/timerErrors"; /** * Owns the save-to-localStorage and save-to-database pipeline for a Core @@ -41,7 +42,7 @@ export class StatePersistence { clearTimeout(this.saveDocStateTimeoutID); } this.saveDocStateTimeoutID = setTimeout(() => { - this.saveState(); + this.saveState().catch(reportTimerError("scheduled saveState")); }, delayMs); } @@ -139,7 +140,9 @@ export class StatePersistence { // check for changes again after 60 seconds this.saveStateToDBTimerId = setTimeout(() => { this.saveStateToDBTimerId = null; - this.saveChangesToDatabase(); + this.saveChangesToDatabase().catch( + reportTimerError("throttled saveChangesToDatabase"), + ); }, 60000); this.core.reportScoreAndStateCallback({ diff --git a/packages/doenetml-worker-javascript/src/StateVariableNameResolver.ts b/packages/doenetml-worker-javascript/src/StateVariableNameResolver.ts index 1ade343aa..b0554daf1 100644 --- a/packages/doenetml-worker-javascript/src/StateVariableNameResolver.ts +++ b/packages/doenetml-worker-javascript/src/StateVariableNameResolver.ts @@ -66,7 +66,7 @@ export function findCaseInsensitiveMatches({ lowerCaseVarName.substring(0, prefix.length) === prefix.toLowerCase() ) { - // TODO: the varEnding is still a case-senstitive match + // TODO: the varEnding is still a case-sensitive match // Should we require that getArrayKeysFromVarName have // a case-insensitive mode? let arrayVariableName = diff --git a/packages/doenetml-worker-javascript/src/VisibilityTracker.ts b/packages/doenetml-worker-javascript/src/VisibilityTracker.ts index 76599b12c..939550a0a 100644 --- a/packages/doenetml-worker-javascript/src/VisibilityTracker.ts +++ b/packages/doenetml-worker-javascript/src/VisibilityTracker.ts @@ -1,3 +1,5 @@ +import { reportTimerError } from "./utils/timerErrors"; + type VisibilityInfo = { componentsCurrentlyVisible: Record; infoToSend: Record; @@ -139,10 +141,11 @@ export class VisibilityTracker { if (this.info.saveTimerId !== null) { clearTimeout(this.info.saveTimerId); } - this.info.saveTimerId = setTimeout( - () => this.sendVisibilityChangedEvents(), - this.info.saveDelay, - ); + this.info.saveTimerId = setTimeout(() => { + this.sendVisibilityChangedEvents()?.catch( + reportTimerError("visibility periodic send"), + ); + }, this.info.saveDelay); } return promise; @@ -169,18 +172,20 @@ export class VisibilityTracker { if (this.info.saveTimerId !== null) { clearTimeout(this.info.saveTimerId); } - this.info.saveTimerId = setTimeout( - () => this.sendVisibilityChangedEvents(), - this.info.saveDelay, - ); + this.info.saveTimerId = setTimeout(() => { + this.sendVisibilityChangedEvents()?.catch( + reportTimerError("visibility resume send"), + ); + }, this.info.saveDelay); } if (this.info.suspendTimerId !== null) { clearTimeout(this.info.suspendTimerId); } - this.info.suspendTimerId = setTimeout( - () => this.suspendVisibilityMeasuring(), - this.info.suspendDelay, - ); + this.info.suspendTimerId = setTimeout(() => { + this.suspendVisibilityMeasuring().catch( + reportTimerError("visibility auto-suspend"), + ); + }, this.info.suspendDelay); } } diff --git a/packages/doenetml-worker-javascript/src/utils/timerErrors.ts b/packages/doenetml-worker-javascript/src/utils/timerErrors.ts new file mode 100644 index 000000000..8f7f679df --- /dev/null +++ b/packages/doenetml-worker-javascript/src/utils/timerErrors.ts @@ -0,0 +1,12 @@ +/** + * Returns a `.catch` handler that logs errors from fire-and-forget Promises + * launched inside `setTimeout` callbacks. Per AGENTS.md, intentionally + * unawaited Promises must attach an explicit rejection handler. + * + * Usage: `asyncCall().catch(reportTimerError("descriptive label"));` + */ +export function reportTimerError(label: string) { + return (error: unknown) => { + console.error(`Error in ${label}:`, error); + }; +} From 454c2aa9236fba4c61e7d90044aec60112ca0057 Mon Sep 17 00:00:00 2001 From: Duane Nykamp Date: Sat, 2 May 2026 23:39:18 -0500 Subject: [PATCH 5/8] refactor(worker-javascript): apply Phase 1 PR review round 2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the in-scope findings from the second-round review of the Core.js helper extraction: - DiagnosticsManager: replace local Diagnostic type with the existing DiagnosticRecord discriminated union from @doenet/utils; preliminary diagnostics now type-narrow via NonErrorDiagnosticRecord, addDiagnostic pushes the record directly, and the dedup check uses explicit branches. - DiagnosticsManager: add markPending() and route the one external setter (Core.js error-component path) through it; drop the set hasPendingDiagnostics accessor on Core. - Core.js: replace five aliased imports with a single import * as nameResolver namespace import. - Core.js: remove six near-duplicate wrapper-block preambles; replace with one file-level explanation and short // → managerName markers. - DiagnosticsManager: tighten getDiagnostics docstring (was misleading about the codebase accumulating messages); correct addDiagnostic return-value docstring (Core.js does inspect it for the rethrow path). - StatePersistence: drop the "slated for Phase 4" reference. - AGENTS.md: clarify Layer 2 Rust phrasing; expand the "Add a new component type" step about schema registration. Defer the structural items (typing core: any, reducing stateless managers to plain functions, moving getSourceLocationForComponent, TimerLabels const, regression test for the .primitive.value fix) to CORE_REFACTOR_DEFERRED.md for the next refactor pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- AGENTS.md | 8 +- CORE_REFACTOR_DEFERRED.md | 67 +++++++++ .../doenetml-worker-javascript/src/Core.js | 71 +++------ .../src/DiagnosticsManager.ts | 138 +++++++++--------- .../src/StatePersistence.ts | 2 +- 5 files changed, 161 insertions(+), 125 deletions(-) create mode 100644 CORE_REFACTOR_DEFERRED.md diff --git a/AGENTS.md b/AGENTS.md index 6b6c21567..7f8c66ab8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -11,8 +11,8 @@ The **parser** (`packages/parser`) converts DoenetML XML into a **DAST** (Docume ### Layer 2: Computation (Worker) The **worker** (`packages/doenetml-worker`) runs in a Web Worker and manages document state and computation. It combines: -- **JavaScript logic** (`packages/doenetml-worker-javascript`) for state updates and interactions -- **Rust/WASM logic** (`packages/doenetml-worker-rust/lib-js-wasm-binding`). The worker is slowly being transitioned to Rust. For the main `packages/doenetml` component, just small pieces of Rust (e.g., reference resolution) are currently invoked. +- **JavaScript logic** (`packages/doenetml-worker-javascript`) for component evaluation, dependency tracking, and state updates +- **Rust/WASM logic** (`packages/doenetml-worker-rust/lib-js-wasm-binding`). Reference-resolution paths run in Rust today; the rest of the worker is slowly being transitioned over. Communication between main thread and worker uses structured messages. The worker is responsible for evaluating components, tracking dependencies (DAG), and managing variants. @@ -166,9 +166,9 @@ The worker receives serialized updates and returns rendered component states. Re ## Common Tasks ### Add a new component type -1. Implement the **worker logic** in `packages/doenetml-worker-javascript` +1. Implement the **worker logic** in `packages/doenetml-worker-javascript` (component class, attributes, state variables, actions) 2. Implement the **UI renderer** in `packages/doenetml/src/Viewer/renderers` or similar -3. Build the schema +3. Register the component in `componentInfoObjects` so the worker knows about it; if the component appears in the DAST/normalized-DAST schema, update the relevant schema definitions too 4. Add **tests** in both Vitest and Cypress 5. Add a **changeset** if user-facing diff --git a/CORE_REFACTOR_DEFERRED.md b/CORE_REFACTOR_DEFERRED.md new file mode 100644 index 000000000..1da921881 --- /dev/null +++ b/CORE_REFACTOR_DEFERRED.md @@ -0,0 +1,67 @@ +# Core.js refactor — deferred findings + +These items came out of the PR review for `core-refactor-1` (Phase 1: extracting helpers from `packages/doenetml-worker-javascript/src/Core.js`). They were intentionally out of scope for that PR but are good candidates for the next refactor pass. + +The applied PR review is at `pr-review.md`. The implementation in this branch covers the in-scope items (#1, #2, #3a, #4, #8, #9, #12, #13). + +## Deferred items + +### Type the `core: any` back-reference in extracted managers + +Every new manager (`DiagnosticsManager`, `VisibilityTracker`, `StatePersistence`, `AutoSubmitManager`, `NavigationHandler`, `ResolverAdapter`) declares `core: any;`. That defeats the point of converting to TypeScript — typos and accidental property reads through `core` go unchecked. + +Since `Core.js` is still JavaScript, defining a real `Core` interface is awkward. Two practical paths: + +1. **A minimal `CoreBackref` interface** in `packages/doenetml-worker-javascript/src/types/coreBackref.ts` listing only the fields each manager actually reads. This is documentary and catches typos. Each manager can narrow further with `Pick`. +2. **Convert `Core.js` to `Core.ts`** in a follow-up phase. Requires real type definitions for `_components`, `flags`, `componentInfoObjects`, etc. — a much larger lift but lets the managers drop their back-reference typing concerns entirely. + +(1) is the cheaper near-term win. + +### Reduce stateless managers to plain functions + +`NavigationHandler` and `ResolverAdapter` hold no state of their own — only a `core` back-reference. The pure-function shape used in `StateVariableNameResolver.ts` is more honest for these: + +```ts +// NavigationHandler.ts +export async function handleNavigatingToComponent({ + core, componentIdx, hash, +}: { core: CoreBackref; componentIdx: number; hash: string }) { ... } + +export function navigateToTarget({ core, args }: { core: CoreBackref; args: any }) { ... } +``` + +Core's wrappers shrink by one line each. If we keep the class form for symmetry with the other (genuinely stateful) managers, that's defensible — but the current PR has both shapes coexisting, so the inconsistency is real. + +### Move `getSourceLocationForComponent` out of `DiagnosticsManager` + +It currently lives at `DiagnosticsManager.ts:150-170`. It walks `_components` ancestors to find a position; nothing about it is diagnostic-specific. It's only there because diagnostics are its primary caller (`Core.js:7530`). Better home: `packages/doenetml-worker-javascript/src/utils/descendants.js` alongside `ancestorsIncludingComposites`. + +### `TimerLabels` constants for `reportTimerError` + +Each call site to `reportTimerError(...)` writes a hand-typed label (`"auto-submit answers"`, `"scheduled saveState"`, `"throttled saveChangesToDatabase"`, `"visibility periodic send"`, `"visibility resume send"`, `"visibility auto-suspend"`). A shared object would catch typos and centralize the namespace: + +```ts +// packages/doenetml-worker-javascript/src/utils/timerErrors.ts +export const TimerLabels = { + autoSubmit: "auto-submit answers", + scheduledSaveState: "scheduled saveState", + throttledSaveChanges: "throttled saveChangesToDatabase", + visibilityPeriodicSend: "visibility periodic send", + visibilityResumeSend: "visibility resume send", + visibilityAutoSuspend: "visibility auto-suspend", +} as const; +``` + +Skip if the next phase introduces a broader logging/instrumentation layer that supersedes this helper. + +### Regression test for the `.primitive.number` → `.primitive.value` fix + +Commit `aed7910c` fixed a latent bug at `ResolverAdapter.ts:84` (was reading `component.attributes.createComponentIdx.primitive.number`, should be `.value` after a `primitive.type === "number"` guard). The fix matches the pattern at `utils/resolver.ts:220-224` and `utils/componentIndices.ts:725`. + +This codepath fires when a copy component (created via `extend`) has a `source:sequence` attribute and a numeric `createComponentIdx`. A targeted Vitest case constructing that specific shape and asserting the resolver receives a `parentSourceSequence` with the correct `parent: ` field would lock the fix in. Without it, regressions could re-introduce the silent `undefined`-parent bug. + +## Notes for the next agent + +- The applied items in this PR are self-contained — see the diff against the previous commit. The structural deferrals above don't depend on each other except that #1 (typing) makes #2 (stateless→plain) cleaner since the back-reference type would already exist. +- AGENTS.md is the source of truth for project conventions; CLAUDE.md is now a one-line pointer. +- Cypress runs in CI; do not include local Cypress steps in any verification plan. diff --git a/packages/doenetml-worker-javascript/src/Core.js b/packages/doenetml-worker-javascript/src/Core.js index 68be61537..1ce8d4eaf 100644 --- a/packages/doenetml-worker-javascript/src/Core.js +++ b/packages/doenetml-worker-javascript/src/Core.js @@ -33,13 +33,7 @@ import { NavigationHandler } from "./NavigationHandler"; import { ResolverAdapter } from "./ResolverAdapter"; import { StatePersistence } from "./StatePersistence"; import { VisibilityTracker } from "./VisibilityTracker"; -import { - findCaseInsensitiveMatches as resolveCaseInsensitiveMatches, - matchPublicStateVariables as resolveMatchPublicStateVariables, - substituteAliases as resolveSubstituteAliases, - publicCaseInsensitiveAliasSubstitutions as resolvePublicCaseInsensitiveAliasSubstitutions, - checkIfArrayEntry as resolveCheckIfArrayEntry, -} from "./StateVariableNameResolver"; +import * as nameResolver from "./StateVariableNameResolver"; import { returnDefaultArrayVarNameFromPropIndex, returnDefaultGetArrayKeysFromVarName, @@ -52,6 +46,14 @@ import { // string to componentClass: this.componentInfoObjects.allComponentClasses["string"] // componentClass to string: componentClass.componentType +// Several feature areas have been extracted into their own modules +// (DiagnosticsManager, VisibilityTracker, StatePersistence, AutoSubmitManager, +// NavigationHandler, ResolverAdapter, and the `nameResolver` namespace). Core +// retains thin wrapper methods so the public surface — used by CoreWorker, +// `coreFunctions`-bound references, components, and tests — keeps working. +// Each delegating block is grouped near its original location and tagged with +// a `// → managerName` marker; see the corresponding module for details. + export default class Core { constructor({ doenetML, @@ -459,11 +461,7 @@ export default class Core { this.updateRenderersCallback({ ...args, init, diagnostics }); } - // Diagnostic state and helpers live in `this.diagnosticsManager` - // (see DiagnosticsManager.ts). The accessors and methods below preserve - // the public surface (`core.diagnostics`, `core.hasPendingDiagnostics`, - // `core.addDiagnostic`, etc.) by delegating through. - + // → diagnosticsManager get diagnostics() { return this.diagnosticsManager.diagnostics; } @@ -472,10 +470,6 @@ export default class Core { return this.diagnosticsManager.hasPendingDiagnostics; } - set hasPendingDiagnostics(value) { - this.diagnosticsManager.hasPendingDiagnostics = value; - } - getDiagnostics() { return this.diagnosticsManager.getDiagnostics(); } @@ -1349,7 +1343,7 @@ export default class Core { // create a serialized component that doesn't exist. const message = `Invalid component type: \`<${serializedComponent.componentType}>\`.`; - this.hasPendingDiagnostics = true; + this.diagnosticsManager.markPending(); const convertResult = convertToErrorComponent( serializedComponent, @@ -2635,10 +2629,7 @@ export default class Core { return { success: true, compositesExpanded: [component.componentIdx] }; } - // Resolver adapter methods live in `this.resolverAdapter` - // (see ResolverAdapter.ts). The methods below preserve the public surface - // (`core.addReplacementsToResolver`, etc.) by delegating through. - + // → resolverAdapter async addReplacementsToResolver(args) { return this.resolverAdapter.addReplacementsToResolver(args); } @@ -7742,14 +7733,11 @@ export default class Core { } } - // The five state-variable name-resolution helpers below live as pure - // functions in StateVariableNameResolver.ts. The wrappers preserve the - // public surface (`core.findCaseInsensitiveMatches`, etc., plus the - // by-reference passes used in composite sugar functions) by injecting - // `componentInfoObjects` and delegating. + // → nameResolver (state-variable name-resolution wrappers inject + // `componentInfoObjects` so external callers don't have to.) findCaseInsensitiveMatches({ stateVariables, componentClass }) { - return resolveCaseInsensitiveMatches({ + return nameResolver.findCaseInsensitiveMatches({ stateVariables, componentClass, componentInfoObjects: this.componentInfoObjects, @@ -7757,7 +7745,7 @@ export default class Core { } matchPublicStateVariables({ stateVariables, componentClass }) { - return resolveMatchPublicStateVariables({ + return nameResolver.matchPublicStateVariables({ stateVariables, componentClass, componentInfoObjects: this.componentInfoObjects, @@ -7765,7 +7753,7 @@ export default class Core { } substituteAliases({ stateVariables, componentClass }) { - return resolveSubstituteAliases({ + return nameResolver.substituteAliases({ stateVariables, componentClass, componentInfoObjects: this.componentInfoObjects, @@ -7776,7 +7764,7 @@ export default class Core { stateVariables, componentClass, }) { - return resolvePublicCaseInsensitiveAliasSubstitutions({ + return nameResolver.publicCaseInsensitiveAliasSubstitutions({ stateVariables, componentClass, componentInfoObjects: this.componentInfoObjects, @@ -7784,7 +7772,7 @@ export default class Core { } checkIfArrayEntry({ stateVariable, component }) { - return resolveCheckIfArrayEntry({ stateVariable, component }); + return nameResolver.checkIfArrayEntry({ stateVariable, component }); } async createFromArrayEntry({ @@ -11276,11 +11264,7 @@ export default class Core { this.sendEvent(payload); } - // Visibility tracking lives in `this.visibilityTracker` - // (see VisibilityTracker.ts). The accessor and methods below preserve - // the public surface (`core.visibilityInfo`, `core.processVisibilityChangedEvent`, - // etc.) by delegating through. - + // → visibilityTracker get visibilityInfo() { return this.visibilityTracker.info; } @@ -12626,11 +12610,7 @@ export default class Core { } } - // State persistence (save to localStorage / database) lives in - // `this.statePersistence` (see StatePersistence.ts). The methods below - // preserve the public surface (`core.saveImmediately`, `core.saveState`, - // `core.saveChangesToDatabase`) by delegating through. - + // → statePersistence async saveImmediately() { return this.statePersistence.saveImmediately(); } @@ -12674,9 +12654,7 @@ export default class Core { } } - // Navigation methods delegate to `this.navigationHandler` - // (see NavigationHandler.ts). - + // → navigationHandler async handleNavigatingToComponent(args) { return this.navigationHandler.handleNavigatingToComponent(args); } @@ -12707,10 +12685,7 @@ export default class Core { await this.saveImmediately(); } - // Auto-submit-answer queue lives in `this.autoSubmitManager` - // (see AutoSubmitManager.ts). The methods below preserve the public surface - // (`core.recordAnswerToAutoSubmit`, `core.autoSubmitAnswers`) by delegating. - + // → autoSubmitManager recordAnswerToAutoSubmit(componentIdx) { this.autoSubmitManager.recordAnswer(componentIdx); } diff --git a/packages/doenetml-worker-javascript/src/DiagnosticsManager.ts b/packages/doenetml-worker-javascript/src/DiagnosticsManager.ts index 81d4787a7..aa3ad6ec2 100644 --- a/packages/doenetml-worker-javascript/src/DiagnosticsManager.ts +++ b/packages/doenetml-worker-javascript/src/DiagnosticsManager.ts @@ -1,26 +1,14 @@ -type DiagnosticType = "error" | "warning" | "info" | "accessibility"; -type DiagnosticLevel = 1 | 2; - -type DiagnosticInput = { - type: DiagnosticType; - level?: DiagnosticLevel; - message: string; - position?: any; - sourceDoc?: number; -}; - -type DiagnosticRecord = { - type: DiagnosticType; - level?: DiagnosticLevel; - message: string; - position?: any; - sourceDoc?: number; -}; - -type AssertableDiagnostic = { - type: DiagnosticType; - level?: DiagnosticLevel; -}; +import { + AccessibilityRecord, + DiagnosticRecord, + InfoRecord, + WarningRecord, +} from "@doenet/utils"; + +type NonErrorDiagnosticRecord = + | WarningRecord + | InfoRecord + | AccessibilityRecord; /** * Owns the diagnostics queue (errors, warnings, info, accessibility) for a Core @@ -39,41 +27,43 @@ export class DiagnosticsManager { preliminaryDiagnostics, }: { core: any; - preliminaryDiagnostics: DiagnosticInput[]; + preliminaryDiagnostics: DiagnosticRecord[]; }) { this.core = core; - this.diagnostics = preliminaryDiagnostics - // Note: we ignore preliminary errors, as we'll gather those from the dast when processing it. - .filter((diagnostic) => diagnostic.type !== "error") - .map((diagnostic) => { + // Preliminary diagnostics seed the queue at construction. We skip the + // dedup pass here — it would be O(n²) and these come from a single + // upstream pass that has already produced unique entries. Errors are + // ignored here; we'll gather those from the dast when processing it. + this.diagnostics = preliminaryDiagnostics.filter( + (diagnostic): diagnostic is NonErrorDiagnosticRecord => { + if (diagnostic.type === "error") { + return false; + } this.assertDiagnosticIsValid(diagnostic); + return true; + }, + ); - return { - type: diagnostic.type, - ...(diagnostic.type === "accessibility" - ? { level: diagnostic.level } - : {}), - message: diagnostic.message, - position: diagnostic.position, - sourceDoc: diagnostic.sourceDoc, - }; - }); + this.hasPendingDiagnostics = true; + } + /** + * Mark the queue as having pending diagnostics without adding a record. + * Used by Core's error-component path, where the diagnostic itself is + * synthesized later by `convertToErrorComponent`. + */ + markPending(): void { this.hasPendingDiagnostics = true; } /** * Get pending diagnostics and reset the pending flag. - * Automatically trims the diagnostics array to prevent unbounded memory growth. * - * @returns Object containing the current diagnostics array - * @note Diagnostics older than the 1000 most recent are discarded to manage memory + * Caps the diagnostics array at the most recent {@link MAX_DIAGNOSTICS} + * entries so a long session with many warnings can't grow it unboundedly. */ getDiagnostics(): { diagnostics: DiagnosticRecord[] } { - // Keep only the last 1000 diagnostics to avoid unbounded memory growth. - // Once the limit is exceeded, older diagnostics are discarded. - // This ensures the codebase doesn't accumulate large numbers of stale messages. const MAX_DIAGNOSTICS = 1000; this.diagnostics = this.diagnostics.slice(-MAX_DIAGNOSTICS); @@ -82,7 +72,13 @@ export class DiagnosticsManager { return { diagnostics: this.diagnostics }; } - assertDiagnosticIsValid({ type, level }: AssertableDiagnostic): void { + assertDiagnosticIsValid({ + type, + level, + }: { + type: DiagnosticRecord["type"]; + level?: number; + }): void { if (!["error", "warning", "info", "accessibility"].includes(type)) { throw Error("Invalid diagnostic type"); } @@ -102,15 +98,12 @@ export class DiagnosticsManager { * Add a diagnostic record to `this.diagnostics`, deduplicating by * type + message + source location. * - * @returns `true` if a new entry was added, `false` if deduped. + * Returns `true` if a new entry was added, `false` if it was deduped. + * Core's initial-add phase inspects this so it can re-throw deduped errors + * via the existing rethrow path (see `Core.js` near the + * `result.sendDiagnostics` loop). */ - addDiagnostic({ - type, - level, - message, - position, - sourceDoc, - }: DiagnosticInput): boolean { + addDiagnostic(diagnostic: DiagnosticRecord): boolean { const sameLocation = (pointA: any, pointB: any) => (pointA?.offset ?? undefined) === (pointB?.offset ?? undefined) && (pointA?.line ?? undefined) === (pointB?.line ?? undefined) && @@ -127,30 +120,31 @@ export class DiagnosticsManager { ); }; - this.assertDiagnosticIsValid({ type, level }); - - const alreadyHaveDiagnostic = this.diagnostics.some( - (diagnostic) => - diagnostic.type === type && - (type === "accessibility" - ? diagnostic.level === level - : true) && - diagnostic.message === message && - diagnostic.sourceDoc === sourceDoc && - haveSamePosition(diagnostic.position, position), - ); + this.assertDiagnosticIsValid(diagnostic); + + const alreadyHaveDiagnostic = this.diagnostics.some((existing) => { + if (existing.type !== diagnostic.type) { + return false; + } + if ( + diagnostic.type === "accessibility" && + existing.type === "accessibility" && + existing.level !== diagnostic.level + ) { + return false; + } + return ( + existing.message === diagnostic.message && + existing.sourceDoc === diagnostic.sourceDoc && + haveSamePosition(existing.position, diagnostic.position) + ); + }); if (alreadyHaveDiagnostic) { return false; } - this.diagnostics.push({ - type, - ...(type === "accessibility" ? { level } : {}), - message, - position, - sourceDoc, - }); + this.diagnostics.push(diagnostic); this.hasPendingDiagnostics = true; return true; diff --git a/packages/doenetml-worker-javascript/src/StatePersistence.ts b/packages/doenetml-worker-javascript/src/StatePersistence.ts index 8cecd65dd..53ed30387 100644 --- a/packages/doenetml-worker-javascript/src/StatePersistence.ts +++ b/packages/doenetml-worker-javascript/src/StatePersistence.ts @@ -16,7 +16,7 @@ import { reportTimerError } from "./utils/timerErrors"; * * This is purely the persistence I/O — the essential-value write engine * that produces `cumulativeStateVariableChanges` is a separate concern - * (see `processNewStateVariableValues` in Core, slated for Phase 4). + * (see `processNewStateVariableValues` in Core). */ export class StatePersistence { core: any; From 8951df56c7055dfa3621b9b45c4cde3bada8934e Mon Sep 17 00:00:00 2001 From: Duane Nykamp Date: Sat, 2 May 2026 23:43:54 -0500 Subject: [PATCH 6/8] refactor(worker-javascript): apply Phase 1 PR review round 3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the three new Copilot comments from the second review pass: - StatePersistence.saveImmediately: replace truthy check on saveDocStateTimeoutID with explicit `!== null`, matching scheduleSave at line 41, and null the field after clearTimeout for self-contained timer state (saveState happens to null it too, but the call site shouldn't depend on that). - AGENTS.md: drop the redundant `-- run` from the test:all-no-worker-js command (the script already appends `-- run` internally), and tighten the description from "all packages except worker" to "all packages except `doenetml-worker-javascript`" — only that one worker package is excluded; doenetml-worker and doenetml-worker-rust are still included. - StateVariableNameResolver.checkIfArrayEntry: typo "begins when an arrayEntry" -> "begins with an array entry". Co-Authored-By: Claude Opus 4.7 (1M context) --- AGENTS.md | 4 ++-- packages/doenetml-worker-javascript/src/StatePersistence.ts | 3 ++- .../src/StateVariableNameResolver.ts | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 7f8c66ab8..9da433a0a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -86,8 +86,8 @@ Read [TEST_RUN_INSTRUCTIONS_FOR_AGENTS.md](TEST_RUN_INSTRUCTIONS_FOR_AGENTS.md) # Run all tests (Vitest only, very slow) npm run test -# Run Vitest only (all packages except worker) -npm run test:all-no-worker-js -- run +# Run Vitest only (all packages except `doenetml-worker-javascript`) +npm run test:all-no-worker-js # Run targeted Vitest (e.g., prefigure package) npm run test -w @doenet/prefigure -- --run test/index-api.test.ts diff --git a/packages/doenetml-worker-javascript/src/StatePersistence.ts b/packages/doenetml-worker-javascript/src/StatePersistence.ts index 53ed30387..a2a4f417c 100644 --- a/packages/doenetml-worker-javascript/src/StatePersistence.ts +++ b/packages/doenetml-worker-javascript/src/StatePersistence.ts @@ -47,11 +47,12 @@ export class StatePersistence { } async saveImmediately(): Promise { - if (this.saveDocStateTimeoutID) { + if (this.saveDocStateTimeoutID !== null) { // if in debounce to save doc to local storage // then immediate save to local storage // and override timeout to save to database clearTimeout(this.saveDocStateTimeoutID); + this.saveDocStateTimeoutID = null; await this.saveState(true); } else { // else override timeout to save any pending changes to database diff --git a/packages/doenetml-worker-javascript/src/StateVariableNameResolver.ts b/packages/doenetml-worker-javascript/src/StateVariableNameResolver.ts index b0554daf1..47538d907 100644 --- a/packages/doenetml-worker-javascript/src/StateVariableNameResolver.ts +++ b/packages/doenetml-worker-javascript/src/StateVariableNameResolver.ts @@ -247,7 +247,7 @@ export function checkIfArrayEntry({ arrayEntryPrefix: string; } | { isArrayEntry: false } { - // check if stateVariable begins when an arrayEntry + // check if stateVariable begins with an array entry for (let arrayEntryPrefix in component.arrayEntryPrefixes) { if ( stateVariable.substring(0, arrayEntryPrefix.length) === From 105075651bed5efb8515eb9c63c10f7306a40c3c Mon Sep 17 00:00:00 2001 From: Duane Nykamp Date: Sun, 3 May 2026 00:03:14 -0500 Subject: [PATCH 7/8] docs: update CORE_REFACTOR_DEFERRED with calcStartEndIdx cleanup item and fix stale reference Co-Authored-By: Claude Sonnet 4.6 --- CORE_REFACTOR_DEFERRED.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/CORE_REFACTOR_DEFERRED.md b/CORE_REFACTOR_DEFERRED.md index 1da921881..3c5319671 100644 --- a/CORE_REFACTOR_DEFERRED.md +++ b/CORE_REFACTOR_DEFERRED.md @@ -2,7 +2,7 @@ These items came out of the PR review for `core-refactor-1` (Phase 1: extracting helpers from `packages/doenetml-worker-javascript/src/Core.js`). They were intentionally out of scope for that PR but are good candidates for the next refactor pass. -The applied PR review is at `pr-review.md`. The implementation in this branch covers the in-scope items (#1, #2, #3a, #4, #8, #9, #12, #13). +The implementation in this branch covers the in-scope items from the PR review. ## Deferred items @@ -54,6 +54,12 @@ export const TimerLabels = { Skip if the next phase introduces a broader logging/instrumentation layer that supersedes this helper. +### Refactor `calcStartEndIdx` out of `determineParentAndIndexResolutionForResolver` + +`ResolverAdapter.determineParentAndIndexResolutionForResolver` contains a nested `async function calcStartEndIdx(replacements)` that mutates three variables from the outer scope (`start_idx`, `end_idx`, and reads `update_start`, `update_end`, `copyComponent`) as side effects. The function is recursive and returns a value, but callers discard the return and rely entirely on the side-effect mutations. This was carried over verbatim from Core.js. + +A cleaner shape: make `calcStartEndIdx` a standalone helper (or a private method on `ResolverAdapter`) that returns `{ start_idx, end_idx }` directly, eliminating the closure mutation. This untangles the logic and makes the function independently testable. + ### Regression test for the `.primitive.number` → `.primitive.value` fix Commit `aed7910c` fixed a latent bug at `ResolverAdapter.ts:84` (was reading `component.attributes.createComponentIdx.primitive.number`, should be `.value` after a `primitive.type === "number"` guard). The fix matches the pattern at `utils/resolver.ts:220-224` and `utils/componentIndices.ts:725`. From 3998d9272cfc7a167964722e99a24112a4c51870 Mon Sep 17 00:00:00 2001 From: Duane Nykamp Date: Sun, 3 May 2026 00:21:18 -0500 Subject: [PATCH 8/8] docs: note pre-existing fire-and-forget calls remaining in Core.js Adds a deferred-items entry listing three intentionally unawaited Promise calls (Core.js:410, 11160, and 449-452) that pre-date Phase 1 and violate the AGENTS.md fire-and-forget convention, with a suggested fix using the reportTimerError helper added in this PR. Co-Authored-By: Claude Opus 4.7 (1M context) --- CORE_REFACTOR_DEFERRED.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CORE_REFACTOR_DEFERRED.md b/CORE_REFACTOR_DEFERRED.md index 3c5319671..0306d69b8 100644 --- a/CORE_REFACTOR_DEFERRED.md +++ b/CORE_REFACTOR_DEFERRED.md @@ -60,6 +60,16 @@ Skip if the next phase introduces a broader logging/instrumentation layer that s A cleaner shape: make `calcStartEndIdx` a standalone helper (or a private method on `ResolverAdapter`) that returns `{ start_idx, end_idx }` directly, eliminating the closure mutation. This untangles the logic and makes the function independently testable. +### Pre-existing fire-and-forget calls still in `Core.js` + +Three intentionally unawaited Promise calls remain in `Core.js`. They pre-date this PR (this refactor did not introduce them, and adding `.catch` handlers was out of scope for a behavior-preserving extraction), but they violate the AGENTS.md convention that fire-and-forget Promises must attach an explicit `.catch(...)` handler. They should be picked up in the next phase that touches Core directly. + +- **`Core.js:410`** — `this.saveState();` inside the `generateDast` epilogue (`if (!this.receivedStateVariableChanges) { this.saveState(); }`). Returns a Promise; rejection is unhandled. +- **`Core.js:11160`** — `this.saveState(true, true);` inside `processNewStateVariableValues` after recording component submissions. Same shape as above. +- **`Core.js:449-452`** — `setTimeout(this.sendVisibilityChangedEvents.bind(this), this.visibilityInfo.saveDelay)` inside `onDocumentFirstVisible()`. The bound function returns `Promise | undefined`; setTimeout discards the return, so any rejection is unhandled. + +Suggested fix in each case: wrap with `.catch(reportTimerError("