diff --git a/AGENTS.md b/AGENTS.md index 50f3cae1f..9da433a0a 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 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. + +### 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 `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 + +# 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` (component class, attributes, state variables, actions) +2. Implement the **UI renderer** in `packages/doenetml/src/Viewer/renderers` or similar +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 + +### 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 new file mode 100644 index 000000000..95161aa18 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,3 @@ +# CLAUDE.md + +See [AGENTS.md](AGENTS.md) for project conventions, architecture, build commands, and agent guidelines. diff --git a/CORE_REFACTOR_DEFERRED.md b/CORE_REFACTOR_DEFERRED.md new file mode 100644 index 000000000..0306d69b8 --- /dev/null +++ b/CORE_REFACTOR_DEFERRED.md @@ -0,0 +1,83 @@ +# 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 implementation in this branch covers the in-scope items from the PR review. + +## 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. + +### 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. + +### 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("