diff --git a/packages/docs/audits/reactive-pipeline-audit-2026-05.md b/packages/docs/audits/reactive-pipeline-audit-2026-05.md new file mode 100644 index 000000000..81e6db516 --- /dev/null +++ b/packages/docs/audits/reactive-pipeline-audit-2026-05.md @@ -0,0 +1,227 @@ +# Reactive Pipeline Audit - May 2026 + +Audit of the Yjs -> React reactive pipeline. Focused on unnecessary work during checklist editing, but covers the full sync layer. + +## Architecture Summary + +``` +Y.Doc + | + |-- reviewsMap.observeDeep() -------> sync.ts (handleReviewsEvents) + | | + | +--> buildStudyFromYMap() per dirty study + | +--> RAF batch -> projectAtoms.setStudy() + | | + | +--> Consumers: + | useStudy() -> StudyCard, ChecklistYjsWrapper + | useAllStudies() -> OverviewTab, ToDoTab, CompletedTab + | useStudyIds() -> AllStudiesTab loop + | + |-- reviewsMap.observeDeep() -------> useChecklistAnswers (useSyncExternalStore) + | | + | +--> serialize() full answers Y.Map + | +--> Consumer: useChecklistViewModel -> checklist editor + | + |-- annotationsMap.observeDeep() ---> useStudyAnnotations (useSyncExternalStore) + | + +--> Consumer: AnnotationSyncManager in PDF viewer +``` + +Two parallel observer chains on the same Yjs data. The sync.ts path feeds list views via atoms. The direct hooks feed the checklist editor and annotation viewer. Both fire on broader changes than they need. + +## Problems + +### P1: `useChecklistAnswers` observes the entire reviews map + +`useChecklistAnswers.ts:81-86` subscribes to `reviewsMap.observeDeep()`. This means editing study A's name triggers a version bump and re-render in study B's checklist editor. Within the same study, any change (PDF upload, annotation add, reviewer assignment) triggers answer re-serialization even though answers didn't change. + +The `getSnapshot` cache prevents returning a new object when version hasn't changed, but it can't prevent the React render cycle that `useSyncExternalStore` initiates when `onStoreChange` fires. And when version does bump, `serialize()` walks the full answers tree and returns a new object every time, even if the underlying data is identical. + +**Impact**: Every Yjs mutation to any study triggers a render cycle in the checklist editor. For keystroke-heavy editing (notes fields), this compounds with the user's own edits. + +**Fix**: Observe the specific checklist's answers Y.Map instead of the whole reviews map. Falls back to a broader observer only when the answers map doesn't exist yet (first load / new checklist). + +### P2: `buildStudyFromYMap` does redundant work + +Two issues here: + +**a) `studyYMap.toJSON()` eagerly serializes everything.** +Line `sync.ts:72` calls `toJSON()` on the full study Y.Map, which recursively serializes all nested Y.Maps (checklists, annotations, PDFs, reconciliation). Then `buildStudyFromYMap` only uses the result for ~20 flat string fields (name, doi, authors, etc.) and re-walks the nested Y.Maps manually to build structured objects. The deep serialization of nested data in `toJSON()` is wasted work. + +**Fix**: Read flat fields directly from the Y.Map (`studyYMap.get('name')` etc.) instead of calling `toJSON()`. Drop the `studyData` parameter from `buildStudyFromYMap`. + +**b) Annotations are serialized but no longer consumed.** +`buildStudyFromYMap` calls `buildAnnotationsFromYMap()` (lines 396-399) which iterates all annotation Y.Maps and parses JSON `embedPdfData` strings. After the `useStudyAnnotations` hook was introduced, `study.annotations` has no remaining consumers -- no component reads it. This is dead work on every sync. + +**Fix**: Stop building annotations in `buildStudyFromYMap`. Remove the `annotations` field from `StudyInfo` if nothing reads it. If something needs it later, add a dedicated hook like `useStudyAnnotations`. + +### P3: Study atoms lack structural equality + +`projectAtoms.ts:29-31` sets the study atom with a new `StudyInfo` object on every sync. The atom has no custom `isEqual`, so `@tldraw/state` uses `Object.is` (reference equality). Since `buildStudyFromYMap` always creates a new object, the atom always notifies subscribers -- even when the user is editing checklist answers and nothing visible in the study card has changed. + +This means every checklist keystroke triggers re-renders in: `StudyCard`, `useAllStudies` consumers (OverviewTab, ToDoTab, CompletedTab, ReconcileTab), and anything else subscribed to that study's atom. + +**Fix**: Add `isEqual` to study atoms. The comparison doesn't need to be deep -- it should compare the fields that list views actually display: name, checklist statuses/scores, reviewer assignments, PDF count, updatedAt. Skip comparing `answers` and `annotations` since those are read through direct hooks. + +### P4: `handlePartialUpdate` doesn't batch across keys + +`ChecklistYjsWrapper.tsx:198-203` iterates `Object.entries(patch)` and calls `updateChecklistAnswer` per key. Each call is now wrapped in its own `ydoc.transact()` (from fix #1), but multiple keys still produce multiple transactions and multiple observer events. + +In practice most patches have a single key, so this is low impact. But when it does fire with multiple keys (e.g., clearing a domain resets judgement + answers), it produces unnecessary intermediate observer events. + +**Fix**: Wrap the loop itself in `ydoc.transact()` so multi-key patches produce a single Yjs event. + +### P5: `useAllStudies` is a computed that reads every study atom + +`projectAtoms.ts:107-119` implements `useAllStudies` as a `useValue` computed that reads `studyOrder` and then `.get()` on every study atom. In `@tldraw/state`, reading an atom inside a computed creates a dependency. So `useAllStudies` re-fires when ANY study atom changes. + +This is correct behavior (it needs the full list), but the consumers -- OverviewTab, ToDoTab, CompletedTab -- all derive filtered subsets from it. When study A's checklist answer changes, all three tabs re-render and re-filter even if the derived list hasn't changed (the study is still in the same tab, with the same status). + +This isn't a bug in `useAllStudies` itself -- it's a consequence of P3. If study atoms had structural equality and didn't fire on answer-only changes, `useAllStudies` would stay stable during checklist editing. + +**Impact**: Medium. The tab views are behind route-based lazy rendering, so only the active tab pays the render cost. But the active tab (usually "All Studies") does re-render on every edit. + +### P6: `updateProjectStats` runs on every sync + +`sync.ts:176` calls `useProjectStore.getState().updateProjectStats(projectId, studies)` on every `doSync()`. This iterates all studies, counts finalized checklists, and writes to localStorage. During active checklist editing this fires on every RAF-batched sync (every ~16ms while typing). + +**Fix**: Only call `updateProjectStats` when checklist status actually changes (FINALIZED count differs), or debounce it separately from the main sync. + +### P7: Score computation on every finalized checklist rebuild + +`buildStudyFromYMap` lines 326-336 call `scoreChecklistOfType()` for every FINALIZED checklist on every study rebuild. Scores for finalized checklists are immutable -- once finalized, answers can't change. + +**Fix**: Cache scores. Either store the computed score in the Y.Map when finalizing (alongside the status change), or cache in the study cache and skip recomputation when the checklist's `updatedAt` hasn't changed. + +## Measurement + +All fixes must be validated with before/after numbers. Add a dev-only performance monitor (gated behind `import.meta.env.DEV`) that logs per-edit-cycle stats to the console. Instrument these hot paths: + +| Probe | Location | What it measures | +| --------------------- | ------------------------------------ | ------------------------------------------------------------------- | +| `handleReviewsEvents` | sync.ts | Fires per Yjs observer event. Count + time. | +| `buildStudyFromYMap` | sync.ts | Full study rebuilds. Count + time per call. | +| `serialize` | useChecklistAnswers.ts `getSnapshot` | Answer re-serializations. Count + cache hit/miss. | +| `doSync` | sync.ts | Store pushes. Count + time. | +| `studyAtom.set` | projectAtoms.ts `setStudy` | Atom notifications. Count + suppressed-by-isEqual count (after P3). | + +Use `performance.mark()` / `performance.measure()` for timing. Aggregate per edit cycle (reset on each `handleReviewsEvents` entry) and log a single summary line: + +``` +[perf] edit cycle: handleReviewsEvents=1 buildStudy=1 serialize=1 doSync=1 atomFired=1 (4.2ms) +``` + +### Measurement protocol + +1. Add instrumentation (P0). +2. Open a checklist with a PDF and annotations loaded. Project should have 5+ studies. +3. Record baseline: click 10 radio buttons, type 20 characters in a notes field. Capture console output. +4. Implement fixes one at a time. After each fix, repeat step 3 and record. +5. Confirm: observer counts drop, serialization counts drop, atom fire counts drop. Wall time should decrease. +6. Remove instrumentation before merging. + +Expected baseline (pre-fix, per radio button click): + +- `handleReviewsEvents`: 2-3 calls (no transaction batching) +- `buildStudyFromYMap`: 2-3 calls +- `serialize`: 2-3 calls +- `doSync`: 1 call (RAF batched) +- `atomFired`: 1+ (always, no isEqual) + +Expected post-fix (per radio button click): + +- `handleReviewsEvents`: 1 call (transaction batching already done) +- `buildStudyFromYMap`: 1 call +- `serialize`: 1 call, or 0 cache hits if observer is narrowed (P1) +- `doSync`: 1 call +- `atomFired`: 0 for answer-only edits (P3 isEqual suppresses) + +## Recommended Execution Order + +The fixes are listed below in order of impact-to-effort ratio. P3 and P1 together eliminate most of the unnecessary work. P2a and P2b are low-effort cleanup. P4-P7 are refinements. + +| Fix | Impact | Effort | Notes | +| ------------------------------------------- | ------ | ------ | --------------------------------------------------------------------- | +| P1: Narrow `useChecklistAnswers` observer | High | Small | Biggest single win for editor performance | +| P3: Add `isEqual` to study atoms | High | Small | Stops cascading re-renders to list views | +| P2b: Stop building annotations in sync | Medium | Small | Dead code removal | +| P2a: Drop `toJSON()` in handleReviewsEvents | Medium | Small | Avoids redundant deep serialization | +| P4: Batch `handlePartialUpdate` | Low | Tiny | Single `transact()` wrapper | +| P6: Gate `updateProjectStats` | Low | Small | Avoids localStorage writes during editing | +| P7: Cache finalized scores | Low | Small | Minor optimization, mostly for studies with many finalized checklists | +| P5: N/A (solved by P3) | - | - | Not a separate fix, just context | + +## Implementation Notes + +### P1 implementation sketch + +```typescript +// useChecklistAnswers.ts subscribe() +const subscribe = useCallback( + (onStoreChange: () => void) => { + if (!ydoc) return () => {}; + const resolved = resolveAnswers(ydoc, studyId, checklistId); + if (!resolved) { + // Answers map doesn't exist yet -- observe the checklist Y.Map + // to catch when answers are first created + const checklistYMap = resolveChecklistYMap(ydoc, studyId, checklistId); + if (!checklistYMap) return () => {}; + const observer = () => { + versionRef.current += 1; + onStoreChange(); + }; + checklistYMap.observe(observer); + return () => checklistYMap.unobserve(observer); + } + const observer = () => { + versionRef.current += 1; + onStoreChange(); + }; + resolved.answersYMap.observeDeep(observer); + return () => resolved.answersYMap.unobserveDeep(observer); + }, + [ydoc, studyId, checklistId], +); +``` + +The subtlety: the observer target can change during the component's lifetime (answers map gets created). `useSyncExternalStore` handles this via the `subscribe` dep array -- when `studyId` or `checklistId` changes, it re-subscribes. + +But there's a lifecycle gap: if the answers Y.Map is created AFTER the initial subscribe (e.g., first edit on a new checklist), the shallow `checklistYMap.observe` catches the creation event, bumps the version, and React calls `getSnapshot` which now resolves the answers. On the next subscribe call (triggered by deps or re-mount), it attaches the deep observer to the now-existing answers map. + +### P3 implementation sketch + +```typescript +// projectAtoms.ts +function studyEquals(a: StudyInfo | undefined, b: StudyInfo | undefined): boolean { + if (a === b) return true; + if (!a || !b) return false; + if (a.name !== b.name) return false; + if (a.updatedAt !== b.updatedAt) return false; + if (a.reviewer1 !== b.reviewer1 || a.reviewer2 !== b.reviewer2) return false; + if (a.pdfs.length !== b.pdfs.length) return false; + if (a.checklists.length !== b.checklists.length) return false; + for (let i = 0; i < a.checklists.length; i++) { + const ca = a.checklists[i], cb = b.checklists[i]; + if (ca.id !== cb.id || ca.status !== cb.status || ca.score !== cb.score + || ca.assignedTo !== cb.assignedTo || ca.updatedAt !== cb.updatedAt) return false; + } + return true; +} + +getOrCreateStudyAtom(studyId: string): Atom { + let a = this.studyAtoms.get(studyId); + if (!a) { + a = atom(`study:${studyId}`, undefined, { + isEqual: studyEquals, + }); + this.studyAtoms.set(studyId, a); + } + return a; +} +``` + +Key decision: include `updatedAt` in the comparison or not. Including it means the atom fires on every edit (since `updateChecklistAnswer` bumps it). Excluding it means list views won't show "last edited" updates in real-time, but the checklist editor doesn't cause cascading re-renders. Recommend excluding it from the equality check and instead using a dedicated "last activity" indicator that reads from Yjs awareness or a separate atom if needed. + +### P2b: removing annotations from StudyInfo + +After removing annotation building from `buildStudyFromYMap`, the `annotations` field on `StudyInfo` becomes dead. Removing it is a cascading change -- the type needs updating and any reference to `study.annotations` needs cleanup. From the search results, no component currently reads it (the only consumer was the useMemo we replaced with `useStudyAnnotations`). The `buildAnnotationsFromYMap` function and the `AnnotationEntry` type may still be needed by `useStudyAnnotations` -- check before removing. diff --git a/packages/web/e2e/flat-key-migration.spec.ts b/packages/web/e2e/flat-key-migration.spec.ts new file mode 100644 index 000000000..207c9d93a --- /dev/null +++ b/packages/web/e2e/flat-key-migration.spec.ts @@ -0,0 +1,284 @@ +/** + * E2E Test: Nested-to-flat Y.Map migration for checklist answers + * + * Verifies that old nested-format Y.Doc data (pre-2026-05-02) is automatically + * migrated to flat dot-notation keys when a project is loaded. The migration + * runs in ConnectionPool after DexieYProvider loads persisted state. + * + * Strategy: + * 1. Create a local checklist and answer questions via the UI + * 2. Rewrite the answers Y.Map to old nested format via page.evaluate + * 3. Reload -- migration runs on the DexieYProvider's persisted state + * 4. Verify answers still display correctly + * + * Uses the dev-mode window.__connectionPool and window.__Y exposures. + * + * Prerequisites: + * pnpm --filter web dev (localhost:3010) + */ + +import { test, expect } from '@playwright/test'; +import { answerAllAMSTAR2, fillROB2Preliminary } from './shared-steps'; + +const TYPE_LABELS: Record = { + AMSTAR2: 'AMSTAR 2', + ROB2: 'RoB 2', +}; + +async function createLocalChecklist( + page: import('@playwright/test').Page, + type: 'AMSTAR2' | 'ROB2', + name: string, +) { + await page.goto('/checklist'); + await expect(page.getByRole('heading', { name: /Start an Appraisal/i })).toBeVisible({ + timeout: 10_000, + }); + + await page.locator('#checklist-type').click(); + await page.getByRole('option', { name: new RegExp(TYPE_LABELS[type]) }).click(); + await page.locator('#checklist-name').fill(name); + await page.getByRole('button', { name: /^Start$/ }).click(); + + await expect(page).toHaveURL(/\/checklist\/[0-9a-f-]{36}/, { timeout: 10_000 }); + await expect(page.getByText('Loading checklist...')).toBeHidden({ timeout: 15_000 }); +} + +/** + * Rewrite a local checklist's flat-key answers to old nested Y.Map format. + * Simulates a production Y.Doc created before the flat-key migration. + */ +async function rewriteAMSTAR2ToNested(page: import('@playwright/test').Page) { + await page.evaluate(() => { + const pool = (window as any).__connectionPool; + const Y = (window as any).__Y; + if (!pool || !Y) throw new Error('Dev-mode globals not available'); + + const entry = pool.getEntry('local-practice'); + if (!entry) throw new Error('No local-practice connection entry'); + + const ydoc = entry.ydoc; + const checklistId = window.location.pathname.split('/').pop()!; + const study = ydoc.getMap('reviews').get(checklistId) as any; + const answersMap = study.get('checklists').get(checklistId).get('answers') as any; + + ydoc.transact(() => { + // Group flat keys by question prefix + const grouped: Record> = {}; + for (const [key, value] of answersMap.entries()) { + const dotIdx = key.indexOf('.'); + if (dotIdx === -1) continue; + const prefix = key.substring(0, dotIdx); + const field = key.substring(dotIdx + 1); + if (!grouped[prefix]) grouped[prefix] = {}; + grouped[prefix][field] = value; + } + + // Clear all flat keys + for (const key of [...answersMap.keys()]) { + answersMap.delete(key); + } + + // Recreate as nested Y.Maps (old format) + for (const [qKey, fields] of Object.entries(grouped)) { + const qMap = new Y.Map(); + if (fields.answers !== undefined) qMap.set('answers', fields.answers); + if (fields.critical !== undefined) qMap.set('critical', fields.critical); + qMap.set('note', new Y.Text()); + answersMap.set(qKey, qMap); + } + }); + }); +} + +async function rewriteROB2ToNested(page: import('@playwright/test').Page) { + await page.evaluate(() => { + const pool = (window as any).__connectionPool; + const Y = (window as any).__Y; + if (!pool || !Y) throw new Error('Dev-mode globals not available'); + + const entry = pool.getEntry('local-practice'); + const ydoc = entry.ydoc; + const checklistId = window.location.pathname.split('/').pop()!; + const study = ydoc.getMap('reviews').get(checklistId) as any; + const answersMap = study.get('checklists').get(checklistId).get('answers') as any; + + const entries: [string, any][] = [...answersMap.entries()]; + + ydoc.transact(() => { + for (const key of [...answersMap.keys()]) { + answersMap.delete(key); + } + + // Build nested preliminary + const preliminary = new Y.Map(); + for (const [key, value] of entries) { + if (!key.startsWith('preliminary.')) continue; + const field = key.substring('preliminary.'.length); + if (value instanceof Y.Text) { + preliminary.set(field, new Y.Text()); + } else { + preliminary.set(field, value); + } + } + answersMap.set('preliminary', preliminary); + + // Build nested domains + const domainAnswers: Record>> = {}; + const domainMeta: Record> = {}; + + for (const [key, value] of entries) { + if (key.startsWith('preliminary.') || key.startsWith('overall.')) continue; + + const dotIdx = key.indexOf('.'); + if (dotIdx === -1) { + // Bare question key like "d1_1" + const match = key.match(/^d(\d+[a-z]?)_/); + if (match) { + const domain = `domain${match[1]}`; + if (!domainAnswers[domain]) domainAnswers[domain] = {}; + if (!domainAnswers[domain][key]) domainAnswers[domain][key] = {}; + domainAnswers[domain][key].answer = value; + } + } else { + const prefix = key.substring(0, dotIdx); + const field = key.substring(dotIdx + 1); + if (prefix.startsWith('domain')) { + if (!domainMeta[prefix]) domainMeta[prefix] = {}; + domainMeta[prefix][field] = value; + } else { + // Question comment like "d1_1.comment" + const match = prefix.match(/^d(\d+[a-z]?)_/); + if (match) { + const domain = `domain${match[1]}`; + if (!domainAnswers[domain]) domainAnswers[domain] = {}; + if (!domainAnswers[domain][prefix]) domainAnswers[domain][prefix] = {}; + domainAnswers[domain][prefix][field] = true; + } + } + } + } + + const allDomains = new Set([...Object.keys(domainAnswers), ...Object.keys(domainMeta)]); + for (const domain of allDomains) { + const domainYMap = new Y.Map(); + const meta = domainMeta[domain] || {}; + if (meta.direction !== undefined) domainYMap.set('direction', meta.direction); + + const answersNested = new Y.Map(); + const qas = domainAnswers[domain] || {}; + for (const [qKey, qVal] of Object.entries(qas)) { + const qYMap = new Y.Map(); + if (qVal.answer !== undefined) qYMap.set('answer', qVal.answer); + qYMap.set('comment', new Y.Text()); + answersNested.set(qKey, qYMap); + } + domainYMap.set('answers', answersNested); + answersMap.set(domain, domainYMap); + } + + // Overall + const overallDirection = entries.find(([k]) => k === 'overall.direction'); + if (overallDirection) { + const overall = new Y.Map(); + overall.set('direction', overallDirection[1]); + answersMap.set('overall', overall); + } + }); + }); +} + +test.describe('Flat-key migration', () => { + test('AMSTAR2: nested answers are migrated and display correctly after reload', async ({ + page, + }) => { + await createLocalChecklist(page, 'AMSTAR2', 'Migration Test AMSTAR2'); + + await answerAllAMSTAR2(page, 'Yes'); + const checkedBefore = await page.getByRole('radio', { name: 'Yes', checked: true }).count(); + expect(checkedBefore).toBeGreaterThan(0); + + // Allow DexieYProvider to persist + await page.waitForTimeout(1000); + + // Rewrite answers to old nested format + await rewriteAMSTAR2ToNested(page); + + // Allow persistence of the nested format + await page.waitForTimeout(1000); + + // Reload -- migrateYDocToFlatKeys runs in ConnectionPool + await page.reload(); + await expect(page.getByText('Loading checklist...')).toBeHidden({ timeout: 15_000 }); + + // All Yes answers should survive the nested -> flat migration + await expect(async () => { + const checkedAfter = await page.getByRole('radio', { name: 'Yes', checked: true }).count(); + expect(checkedAfter).toBe(checkedBefore); + }).toPass({ timeout: 10_000 }); + }); + + test('ROB2: nested preliminary and domain answers migrate correctly', async ({ page }) => { + await createLocalChecklist(page, 'ROB2', 'Migration Test ROB2'); + + await fillROB2Preliminary(page, 'Drug X', 'Placebo'); + + // Select some domain answers + await page.getByRole('button', { name: 'D1', exact: true }).click(); + await expect(page.getByRole('button', { name: 'Y', exact: true }).first()).toBeVisible({ + timeout: 5_000, + }); + const yButtons = page.getByRole('button', { name: 'Y', exact: true }); + const d1Count = await yButtons.count(); + for (let i = 0; i < d1Count; i++) { + await yButtons.nth(i).click(); + await page.waitForTimeout(50); + } + + await page.waitForTimeout(1000); + + // Count selected answers before rewrite + const selectedBefore = await countSelectedToggleButtons(page, 'Y'); + + // Rewrite to nested format (Y.Text content is lost in the rewrite since + // we create new instances, but structural data like study design, aim, + // and signalling question answers preserve their primitive values) + await rewriteROB2ToNested(page); + await page.waitForTimeout(1000); + + // Reload -- migration runs + await page.reload(); + await expect(page.getByText('Loading checklist...')).toBeHidden({ timeout: 15_000 }); + + // The form should render without errors + await expect(page.getByRole('button', { name: 'D1', exact: true })).toBeVisible({ + timeout: 10_000, + }); + + // Navigate to D1 and verify signalling question answers survived + await page.getByRole('button', { name: 'D1', exact: true }).click(); + await expect(async () => { + const selected = await countSelectedToggleButtons(page, 'Y'); + expect(selected).toBe(selectedBefore); + }).toPass({ timeout: 10_000 }); + + // Verify preliminary scalar fields survived (study design radio) + const studyDesignRadio = page.getByText('Individually-randomized parallel-group trial'); + await expect(studyDesignRadio).toBeVisible(); + }); +}); + +async function countSelectedToggleButtons( + page: import('@playwright/test').Page, + name: string, +): Promise { + const buttons = page.getByRole('button', { name, exact: true }); + const count = await buttons.count(); + let selected = 0; + for (let i = 0; i < count; i++) { + const ariaPressed = await buttons.nth(i).getAttribute('aria-pressed'); + const dataState = await buttons.nth(i).getAttribute('data-state'); + if (ariaPressed === 'true' || dataState === 'on') selected++; + } + return selected; +} diff --git a/packages/web/src/components/checklist/AMSTAR2Checklist/AMSTAR2Checklist.tsx b/packages/web/src/components/checklist/AMSTAR2Checklist/AMSTAR2Checklist.tsx index 1a9936331..4ccec0c6a 100644 --- a/packages/web/src/components/checklist/AMSTAR2Checklist/AMSTAR2Checklist.tsx +++ b/packages/web/src/components/checklist/AMSTAR2Checklist/AMSTAR2Checklist.tsx @@ -1,30 +1,17 @@ -/** - * AMSTAR2Checklist - Full AMSTAR2 appraisal form with 16 questions - * - * Each question has a unique column structure with checkboxes (evidence criteria) - * and a final radio column (verdict: Yes/Partial Yes/No). Checking criteria in - * earlier columns automatically derives the verdict in the last column. - * - * Supports both controlled mode (externalChecklist + onExternalUpdate from Yjs) - * and uncontrolled mode (standalone with local state). - */ - -import { useState, useCallback, useMemo } from 'react'; -import type * as Y from 'yjs'; +import { useMemo } from 'react'; import { InfoIcon } from 'lucide-react'; import { AMSTAR_CHECKLIST } from './checklist-map'; -import { createChecklist as createAMSTAR2Checklist } from './checklist.js'; import { Tooltip, TooltipTrigger, TooltipContent } from '@/components/ui/tooltip'; import { NoteEditor } from '@/components/checklist/common/NoteEditor'; -import type { TextRef } from '@/primitives/useProject/checklists'; -import type { - AMSTAR2Checklist as AMSTAR2ChecklistType, - AMSTAR2QuestionAnswer, -} from '@corates/shared/checklists'; +import { + useAnswer, + useAnswersYMap, + useChecklistField, + useProjectReactor, +} from '@/primitives/useProject/reactor/hooks'; +import { resolveYText } from '@/primitives/useProject/reactor/ytext'; import type { AMSTAR2QuestionSchema, AMSTAR2Column } from '@corates/shared/checklists/amstar2'; -// -- Shared internal components -- - function QuestionInfo({ question }: { question: AMSTAR2QuestionSchema }) { return (
@@ -44,38 +31,43 @@ function QuestionInfo({ question }: { question: AMSTAR2QuestionSchema }) { } function CriticalButton({ - state, - onUpdate, + studyId, + checklistId, + qKey, }: { - state: AMSTAR2QuestionAnswer; - onUpdate: (_newState: AMSTAR2QuestionAnswer) => void; + studyId: string; + checklistId: string; + qKey: string; }) { + const critical = useAnswer(studyId, checklistId, `${qKey}.critical`) ?? false; + const answersYMap = useAnswersYMap(studyId, checklistId); + return (
); } -function StandardQuestionInternal({ - state, +function ColumnsGrid({ + answers, question, columns, handleChange, width, }: { - state: AMSTAR2QuestionAnswer; + answers: boolean[][]; question: { text: string }; columns?: AMSTAR2Column[]; handleChange: (_colIdx: number, _optIdx: number) => void; @@ -109,7 +101,7 @@ function StandardQuestionInternal({ handleChange(colIdx, optIdx)} className='border-border focus:ring-primary size-3.5 cursor-pointer text-blue-600' /> @@ -122,7 +114,7 @@ function StandardQuestionInternal({