From cde1be2c6aa255569ffcc700be82cff03bd5ac1f Mon Sep 17 00:00:00 2001 From: Jacob Maynard Date: Sat, 28 Mar 2026 21:29:47 -0500 Subject: [PATCH 1/7] phase 1 --- .../audits/yjs-sync-pipeline-redesign-b.md | 781 ++++++++++++------ .../landing/src/primitives/useProject/sync.ts | 202 ++++- .../landing/src/project/ConnectionPool.ts | 17 +- packages/landing/src/stores/projectStore.ts | 24 +- 4 files changed, 707 insertions(+), 317 deletions(-) diff --git a/packages/docs/audits/yjs-sync-pipeline-redesign-b.md b/packages/docs/audits/yjs-sync-pipeline-redesign-b.md index 019166c9b..00c3abc56 100644 --- a/packages/docs/audits/yjs-sync-pipeline-redesign-b.md +++ b/packages/docs/audits/yjs-sync-pipeline-redesign-b.md @@ -1,375 +1,612 @@ -# Y.js Sync Pipeline Architecture Redesign +# Y.js Sync Pipeline: Targeted Fixes -## Context +Alternative to `yjs-sync-pipeline-redesign-a.md`. Same problems, different approach: fix the bottlenecks instead of replacing the architecture. -The CoRATES Y.js sync pipeline has a critical architectural problem: every keystroke in a collaborative text field triggers **full Y.Doc tree extraction** (505 lines, O(n) traversal) followed by **JSON.stringify equality checks** (O(n) comparison) before pushing to a monolithic Zustand store. This makes the sync layer untestable, untyped, and slow for large projects. +**Status**: Draft +**Date**: 2026-03-28 +**Scope**: `packages/landing` sync infrastructure +**Replaces**: `yjs-sync-pipeline-redesign-a.md` -Research into Zero Sync, TanStack DB, Convex, and Outline revealed that every mature sync library avoids this pattern. The redesign applies their key lessons: +--- -- **Observe subtrees, not the whole doc** (Zero's IVM, Y.js native observeDeep) -- **Only extract what changed** (TanStack DB's per-collection differential dataflow) -- **Reference equality, not deep comparison** (Convex's shallow comparison) -- **Direct Y.Doc access for editors** (Outline's approach -- Y.Doc IS the state for editors) -- **Type safety end-to-end** (all four libraries) +## Table of Contents -## Architecture Overview +1. [Critique of the Redesign RFC](#critique-of-the-redesign-rfc) +2. [Approach](#approach) +3. [Fix 1: Scoped Observation](#fix-1-scoped-observation) +4. [Fix 2: Per-Study Dirty Tracking](#fix-2-per-study-dirty-tracking) +5. [Fix 3: Type the Operations Interface](#fix-3-type-the-operations-interface) +6. [Fix 4: Remove Dead Indirection](#fix-4-remove-dead-indirection) +7. [What Does Not Change](#what-does-not-change) +8. [Migration Plan](#migration-plan) +9. [Risk Assessment](#risk-assessment) -Two independent improvements, implementable in parallel: +--- -### Part A: Reactive Collection Layer (replaces sync.ts) -### Part B: Typed Operations Layer (replaces untyped ops map) +## Critique of the Redesign RFC ---- +The redesign RFC (`yjs-sync-pipeline-redesign-a.md`) correctly identifies these problems: -## Part A: Reactive Collection Layer +1. Full-tree extraction on every Y.Doc change (O(n) per keystroke) +2. Type erasure in `buildOpsMap` / `ConnectionOps` +3. `ConnectionPool` as a god object +4. `JSON.stringify` equality as an O(n) band-aid -Replaces `sync.ts` (505 lines) + the extraction half of `projectStore.ts`. +But the proposed solution -- domain collections, typed handles, a three-layer architecture (Session/Document/React), six migration phases touching 36+ files and 73 components -- has problems of its own: -### Core Abstraction: YCollection\ +**The handles are indirection, not abstraction.** Every method on `ChecklistHandle` is a 1:1 passthrough to the underlying ops. The "type safety" argument is weak because the underlying ops already have typed interfaces (`ChecklistOperations`). The fix for `buildOpsMap` erasing those types is to stop erasing them, not to add a wrapper class. -A framework-agnostic class that observes a single Y.Map subtree via `observeDeep` and maintains a snapshot cache indexed by key. +**The core performance claim is overstated.** The RFC goes from "rebuild all studies" to "rebuild one study" and presents it as O(1). But `buildStudySnapshot` still iterates all checklists, PDFs, and annotations for the changed study. A keystroke in a checklist answer triggers a full study rebuild including siblings. The actual implementation proved this: `StudiesCollection` has `if (!this.snapshots.has(studyId) || true)` -- the `|| true` means path-based diffing was abandoned and every event triggers a full study rebuild anyway. -```typescript -// packages/landing/src/lib/ycollections/collection.ts -class YCollection { - constructor(options: { - rootMap: Y.Map; - extract: (key: string, ymap: Y.Map) => T; - shouldExtract?: (key: string, ymap: Y.Map) => boolean; - }); - - getSnapshot(): ReadonlyMap; // stable ref if unchanged - get(key: string): T | undefined; // stable ref per-entry - getArray(compareFn?): readonly T[]; // stable ref if unchanged - subscribe(listener: () => void): () => void; - destroy(): void; - pause(): void; // for Dexie state loading - resume(): void; -} -``` +**`observeDeep` path assumptions are fragile.** The RFC acknowledges this as "medium risk" but underestimates it. Multi-key transactions produce multiple events with different paths that need careful deduplication. The implementation fell back to always rebuilding, which defeats the stated purpose. -**How change detection works:** -1. `rootMap.observeDeep(handler)` fires with `Y.YEvent[]` -2. Handler extracts affected top-level keys from `event.path[0]` -3. Only those keys are re-extracted (not all entries) -4. New snapshot compared to cached one -- if unchanged, no notification -5. Subscribers notified synchronously after Y.js transaction +**Reference equality leaks through arrays.** Even if study-456's snapshot object is stable, the `sortedList` array that `useStudies()` returns is invalidated when study-123 changes (the RFC nulls it and rebuilds). Any component mapping over studies re-renders because the array reference changed. This is the same problem the RFC criticizes in the current design, moved one level down. -**Three collections per project:** +**The migration is a rewrite disguised as incremental.** Six phases, 11 deleted files, every component touched. Phase 3 (handles) is pure indirection until Phase 4 absorbs the factories. Between phases 3 and 4 there are three ways to access data (Zustand selectors, connectionPool ops, handle wrappers). If the migration stalls, you have added complexity without removing any. -| Collection | Y.Map source | Snapshot type | Volatility | -|---|---|---|---| -| `studies` | `ydoc.getMap('reviews')` | `StudySnapshot` | High (keystroke-level for active editors) | -| `members` | `ydoc.getMap('members')` | `MemberSnapshot` | Low (changes on member add/remove) | -| `meta` | `ydoc.getMap('meta')` | `MetaSnapshot` | Low (project rename, outcome changes) | +**`ProjectDocument` replaces one god object with another.** It owns collections (reactive reads), handles (typed mutations), and project-level commands. Same responsibilities as `ConnectionPool`, different name. -**Key optimization -- skip in-progress answer extraction:** +--- -The current sync.ts extracts checklist answers only for FINALIZED checklists. The collection extractor preserves this: in-progress checklists get metadata-only snapshots (type, status, assignedTo, updatedAt). Editors access in-progress answers directly via Y.Doc handlers (`getChecklistAnswersMap`, `getTextRef`), which already bypass the store. +## Approach -This means the most common event (typing in a checklist) triggers only a metadata-level extraction for that one study, not a full tree traversal. +Instead of replacing the architecture, fix the four specific bottlenecks: -### Extractors (ported from sync.ts) +| Problem | Fix | Files Touched | +|---------|-----|---------------| +| Full-tree extraction on every change | Scoped `observeDeep` per top-level Y.Map | `sync.ts`, `ConnectionPool.ts` | +| O(n) rebuild within `doSync` | Per-study snapshot cache with dirty tracking | `sync.ts` | +| `JSON.stringify` equality | Reference equality via cached snapshots | `projectStore.ts` | +| Type erasure in `buildOpsMap` | Delete `buildOpsMap`, expose typed ops directly | `ConnectionPool.ts`, consumers of `.get()` | +| Unnecessary indirection layers | Delete thin action wrappers, delete handles | Action modules, handle files | -``` -packages/landing/src/lib/ycollections/extractors/ - study.ts -- extractStudy(id, ymap) -> StudySnapshot (port of buildStudyFromYMap) - member.ts -- extractMember(id, ymap) -> MemberSnapshot (port of buildMembersList, per-entry) - meta.ts -- extractMeta(ymap) -> MetaSnapshot (port of meta extraction) - checklist-answers/ - rob2.ts -- extractROB2Answers(answersMap) -> Record - robins-i.ts -- extractROBINSIAnswers(answersMap) -> Record - amstar2.ts -- extractAMSTAR2Answers(answersMap) -> Record +Total: ~5 files significantly changed, ~5 thin files deleted. Zero component API changes for the performance fixes. Typed ops require updating consumers of `connectionPool.get()` (15 files, mechanical find-and-replace). + +--- + +## Fix 1: Scoped Observation + +### Problem + +`ConnectionPool.initializeConnection` binds a single handler to `ydoc.on('update')`: + +```typescript +// ConnectionPool.ts:119-125 +const syncUpdateHandler = () => { + if (!entry.isLoadingPersistedState) { + entry.syncManager?.syncFromYDoc(); + } +}; +ydoc.on('update', syncUpdateHandler); ``` -The 170-line `extractAnswersFromYMap` with 95% ROB2/ROBINS-I duplication splits into one file per checklist type. Each is a pure function: Y.Map in, plain JS out. Testable with simple Y.Doc fixtures. +This fires on every Y.Doc change, triggering `doSync()` which walks the entire tree: all studies, all checklists, all PDFs, all annotations, all members, all meta. -### Integration Strategy (Zustand Bridge) +### Fix -**Phase 1: Collections feed into existing Zustand store.** All 12+ store selector consumers continue working unchanged. +Replace the single `ydoc.on('update')` handler with three scoped observers in `createSyncManager`: ```typescript -// packages/landing/src/lib/ycollections/zustand-bridge.ts -function createZustandBridge(projectId: string, collections: ProjectCollections): () => void { - // Subscribe to each collection independently - // On change, call useProjectStore.getState().setProjectData(projectId, { studies }) - // Only the changed domain is passed (studies OR members OR meta) +export function createSyncManager(projectId: string, getYDoc: () => Y.Doc | null): SyncManager { + let pendingSync = false; + let dirtySlices = { studies: false, members: false, meta: false }; + + function attach(ydoc: Y.Doc): (() => void)[] { + const reviewsMap = ydoc.getMap('reviews'); + const membersMap = ydoc.getMap('members'); + const metaMap = ydoc.getMap('meta'); + + const onReviews = () => { + dirtySlices.studies = true; + scheduleSync(); + }; + const onMembers = () => { + dirtySlices.members = true; + scheduleSync(); + }; + const onMeta = () => { + dirtySlices.meta = true; + scheduleSync(); + }; + + reviewsMap.observeDeep(onReviews); + membersMap.observe(onMembers); // flat, no need for observeDeep + metaMap.observeDeep(onMeta); + + return [ + () => reviewsMap.unobserveDeep(onReviews), + () => membersMap.unobserve(onMembers), + () => metaMap.unobserveDeep(onMeta), + ]; + } + + function scheduleSync(): void { + if (pendingSync) return; + pendingSync = true; + requestAnimationFrame(() => { + pendingSync = false; + doSync(); + }); + } + + function doSync(): void { + const ydoc = getYDoc(); + if (!ydoc) return; + + const updates: Partial = {}; + + if (dirtySlices.studies) { + updates.studies = syncStudies(ydoc); + } + if (dirtySlices.members) { + updates.members = buildMembersList(ydoc.getMap('members')); + } + if (dirtySlices.meta) { + updates.meta = syncMeta(ydoc); + } + + // Reset dirty flags before store update + dirtySlices = { studies: false, members: false, meta: false }; + + if (Object.keys(updates).length > 0) { + useProjectStore.getState().setProjectData(projectId, updates); + } + } + + // ... } ``` -**Phase 2: Direct `useSyncExternalStore` hooks.** Components migrate incrementally from `useProjectStore(selectStudies)` to `useStudies(projectId)`. +When a checklist answer changes, only `dirtySlices.studies` is set. Members and meta are never rebuilt. The RAF batching remains -- if multiple slices change within the same frame, they are combined into one `setProjectData` call. + +### Why scoped observe instead of `ydoc.on('update')` + +`ydoc.on('update')` provides a binary diff (Uint8Array). You cannot determine what changed without decoding it. Y.Map `observeDeep` provides structured events with paths that identify the changed subtree. This is what enables Fix 2. + +### ConnectionPool change + +Remove the `ydoc.on('update', syncUpdateHandler)` binding from `initializeConnection`. Instead, `createSyncManager` calls `attach(ydoc)` internally and returns cleanup functions. The `isLoadingPersistedState` guard moves into the sync manager (the sync manager can expose a `pause()`/`resume()` pair or an `isLoading` flag). + +--- + +## Fix 2: Per-Study Dirty Tracking + +### Problem + +Even with scoped observation (Fix 1), `syncStudies` still walks all studies: ```typescript -// packages/landing/src/hooks/useStudies.ts -function useStudies(projectId: string): readonly StudySnapshot[] { - const collections = useProjectCollections(projectId); - return useSyncExternalStore( - collections.studies.subscribe, - () => collections.studies.getArray(byCreatedAt), - ); +// Current: iterates every study on every change +for (const [studyId, studyYMap] of studiesMap.entries()) { + studiesList.push(buildStudyFromYMap(studyId, ...)); } ``` -### Wiring into ConnectionPool +This is the actual hotspot. A project with 50 studies rebuilds all 50 snapshots when one checklist answer changes. + +### Fix + +Maintain a `Map` cache inside the sync manager. Use `observeDeep` event paths to identify which studies changed. Only rebuild dirty studies. ```typescript -// In ConnectionPool.initializeConnection -- replaces SyncManager creation: -const collections = createProjectCollections(ydoc); -entry.collections = collections; -const teardownBridge = createZustandBridge(projectId, collections); -entry._cleanupHandlers.push(() => { teardownBridge(); collections.destroy(); }); +const studyCache = new Map(); +let sortedStudies: StudyInfo[] = []; + +function handleReviewsEvents(events: Y.YEvent[]): void { + const reviewsMap = getYDoc()?.getMap('reviews'); + if (!reviewsMap) return; + + const dirtyStudyIds = new Set(); + let structuralChange = false; // study added or removed + + for (const event of events) { + if (event.path.length === 0 && 'keys' in event) { + // Top-level: studies added/removed + structuralChange = true; + for (const [key, info] of (event as Y.YMapEvent).keys) { + dirtyStudyIds.add(key); + } + } else if (event.path.length > 0) { + // Nested: something inside a specific study changed + dirtyStudyIds.add(event.path[0] as string); + } + } + + // Rebuild only dirty studies + for (const studyId of dirtyStudyIds) { + const studyYMap = reviewsMap.get(studyId) as Y.Map | undefined; + if (studyYMap) { + const studyData = studyYMap.toJSON ? studyYMap.toJSON() : {}; + studyCache.set(studyId, buildStudyFromYMap(studyId, studyData, studyYMap)); + } else { + studyCache.delete(studyId); + } + } + + // Handle removals: clean up cached entries for deleted studies + if (structuralChange) { + for (const cachedId of studyCache.keys()) { + if (!reviewsMap.has(cachedId)) { + studyCache.delete(cachedId); + } + } + } + + // Rebuild sorted array only if something changed + if (dirtyStudyIds.size > 0 || structuralChange) { + sortedStudies = [...studyCache.values()] + .sort((a, b) => (a.createdAt || 0) - (b.createdAt || 0)); + } +} + +function syncStudies(ydoc: Y.Doc): StudyInfo[] { + // On the first sync (cache empty), populate from scratch + if (studyCache.size === 0) { + const reviewsMap = ydoc.getMap('reviews'); + for (const [studyId, studyYMap] of reviewsMap.entries()) { + const ymap = studyYMap as Y.Map; + const studyData = ymap.toJSON ? ymap.toJSON() : {}; + studyCache.set(studyId, buildStudyFromYMap(studyId, studyData, ymap)); + } + sortedStudies = [...studyCache.values()] + .sort((a, b) => (a.createdAt || 0) - (b.createdAt || 0)); + } + return sortedStudies; +} ``` -The `ydoc.on('update', syncUpdateHandler)` global handler is removed. Collections use `observeDeep` internally. +### How dirty tracking and scoped observation interact -### Files to Create +The `observeDeep` callback on `reviews` receives the events array directly. The callback both (a) extracts dirty study IDs from `event.path` and (b) sets `dirtySlices.studies = true`. When `doSync()` fires on the next RAF, it calls `syncStudies()` which returns the already-updated `sortedStudies` array. -| File | Purpose | Lines (est.) | -|---|---|---| -| `lib/ycollections/collection.ts` | YCollection\ class | ~120 | -| `lib/ycollections/singleton.ts` | YSingleton\ for meta (single map, not keyed) | ~60 | -| `lib/ycollections/types.ts` | StudySnapshot, MemberSnapshot, MetaSnapshot, etc. | ~100 | -| `lib/ycollections/extractors/study.ts` | Port of buildStudyFromYMap | ~80 | -| `lib/ycollections/extractors/member.ts` | Port of buildMembersList (per-entry) | ~20 | -| `lib/ycollections/extractors/meta.ts` | Port of meta extraction | ~30 | -| `lib/ycollections/extractors/checklist-answers/rob2.ts` | ROB2 answer extraction | ~60 | -| `lib/ycollections/extractors/checklist-answers/robins-i.ts` | ROBINS-I answer extraction | ~55 | -| `lib/ycollections/extractors/checklist-answers/amstar2.ts` | AMSTAR2 answer extraction | ~15 | -| `lib/ycollections/project-collections.ts` | Factory: createProjectCollections(ydoc) | ~40 | -| `lib/ycollections/zustand-bridge.ts` | Feeds collections into existing store | ~40 | -| `lib/ycollections/index.ts` | Barrel export | ~10 | -| `hooks/useStudies.ts` | useSyncExternalStore hook | ~15 | -| `hooks/useStudy.ts` | Single study hook | ~15 | -| `hooks/useProjectCollections.ts` | Context hook for collections | ~20 | - -### Files to Modify - -| File | Change | -|---|---| -| `project/ConnectionPool.ts` | Add `collections` to ConnectionEntry, wire in `initializeConnection`, remove `ydoc.on('update')` handler and SyncManager creation | -| `stores/projectStore.ts` | Remove JSON.stringify equality checks (bridge handles change detection); can later remove studies/members/meta fields entirely | - -### Files to Delete (after full migration) - -| File | Reason | -|---|---| -| `primitives/useProject/sync.ts` | Replaced entirely by ycollections | +More precisely: + +1. Y.Doc transaction commits. +2. `reviewsMap.observeDeep` fires synchronously with events. +3. `handleReviewsEvents(events)` rebuilds only dirty studies in `studyCache`, updates `sortedStudies`. +4. The same callback sets `dirtySlices.studies = true` and calls `scheduleSync()`. +5. On the next animation frame, `doSync()` sees `dirtySlices.studies === true`, calls `syncStudies()` which returns the precomputed `sortedStudies`. +6. Store receives the update. Unchanged studies have stable object references. + +### Reference equality follows naturally + +When study-123's checklist answer changes: +- `buildStudyFromYMap` creates a new `StudyInfo` object for study-123 +- `studyCache.get('study-456')` returns the exact same object reference as before +- `sortedStudies` is a new array (unavoidable), but the individual study objects inside it have stable references for unchanged studies +- `selectStudy(state, projectId, 'study-456')` returns the same object -- React skips the re-render + +This is the same granularity the redesign RFC claims, without collections, without `useSyncExternalStore`, without changing any component code. + +### The sorted array problem + +The `sortedStudies` array is a new reference on every change. Components that call `selectStudies` and iterate the array will re-render. This is the same as today. + +Two mitigations, both optional: +1. Components that render individual studies should use `selectStudy(state, projectId, studyId)` instead of mapping over `selectStudies`. This already works with the current selector. +2. If array stability becomes a measured problem, the store setter can check whether the array contents actually changed (shallow element comparison, not `JSON.stringify`). + +Neither is required for the initial fix. The big win is not rebuilding 49 untouched studies on every keystroke. + +### What about sub-study granularity? + +The redesign RFC's `StudiesCollection` also rebuilds the full study on any nested change. The implementation confirmed this with the `|| true` guard. Going finer (per-checklist dirty tracking) is possible but not worth the complexity: + +- Most components that display study data need multiple fields (name, checklists, reviewers). Per-checklist caching would mean reassembling the study view from fragments. +- The actual cost of `buildStudyFromYMap` for one study is small (iterate 2-3 checklists, a few PDFs). The problem was doing it for 50 studies, not for one. +- If profiling later shows single-study rebuilds are slow for studies with many checklists, the cache can be extended to per-checklist snapshots within the study without any API changes. --- -## Part B: Typed Operations Layer +## Fix 3: Type the Operations Interface -Replaces the untyped `ConnectionOps = Record any>` and the 60-line `buildOpsMap` method. +### Problem -### ProjectOps Interface +`ConnectionPool.buildOpsMap()` returns `Record any>`, actively destroying the type information that the operation factories already provide: ```typescript -// packages/landing/src/project/ProjectOps.ts -import type { StudyOperations } from '@/primitives/useProject/studies'; -import type { ChecklistOperations } from '@/primitives/useProject/checklists/index'; -import type { PdfOperations } from '@/primitives/useProject/pdfs'; -import type { ReconciliationOperations } from '@/primitives/useProject/reconciliation'; -import type { AnnotationOperations } from '@/primitives/useProject/annotations'; -import type { OutcomeOperations } from '@/primitives/useProject/outcomes'; - -export interface ProjectOps { - readonly study: StudyOperations; - readonly checklist: ChecklistOperations; - readonly pdf: PdfOperations; - readonly reconciliation: ReconciliationOperations; - readonly annotation: AnnotationOperations; - readonly outcome: OutcomeOperations; +// ConnectionPool.ts:310-311 +private buildOpsMap(entry: ConnectionEntry): ConnectionOps { + const chk = entry.checklistOps as any; + return { + createChecklist: (...args: any[]) => chk?.createChecklist(...args), + // ... 30 more lines of the same pattern + }; } ``` -No new types invented -- reuses the 6 existing typed interfaces. +The factories are already typed. `ChecklistOperations` has full signatures. `buildOpsMap` takes those typed functions and wraps each one in an `any`-typed lambda. -### Accessor API +### Fix + +Delete `buildOpsMap`, delete `opsRegistry`, delete the `ConnectionOps` type. Expose the typed operations directly from `ConnectionEntry`. + +**Step 1: Define a typed operations interface.** ```typescript -// Added to ConnectionPool -getTypedOps(projectId: string): ProjectOps // throws NoConnectionError -getActiveTypedOps(): ProjectOps // throws NoConnectionError +// ConnectionPool.ts + +export interface TypedProjectOps { + study: StudyOperations; + checklist: ChecklistOperations; + pdf: PdfOperations; + reconciliation: ReconciliationOperations; + annotation: AnnotationOperations; + outcome: OutcomeOperations; + getAwareness: () => Awareness | null; +} ``` -These throw instead of returning null because every single existing consumer already throws on null. This eliminates ~35 `if (!ops) throw` guards across the codebase. +Each of these interfaces already exists in the respective factory file (`ChecklistOperations` in `checklists/index.ts`, etc.). If some are missing, define them in the factory file where the implementation lives. -### Consumer Migration (before/after) +**Step 2: Replace `get()` with typed access.** -**Actions (Pattern A, ~35 call sites):** ```typescript -// Before: -const ops = connectionPool.getActiveOps(); -if (!ops) throw new Error('No active project connection'); -ops.createOutcome(name, user.id); // no type checking - -// After: -const { outcome } = connectionPool.getActiveTypedOps(); -outcome.createOutcome(name, user.id); // fully typed +// Before +get(projectId: string): ConnectionOps | null { + return this.opsRegistry.get(projectId) || null; +} + +// After +getOps(projectId: string): TypedProjectOps | null { + const entry = this.registry.get(projectId); + if (!entry?.initialized) return null; + return { + study: entry.studyOps, + checklist: entry.checklistOps!, + pdf: entry.pdfOps, + reconciliation: entry.reconciliationOps, + annotation: entry.annotationOps, + outcome: entry.outcomeOps, + getAwareness: () => entry.connectionManager?.getAwareness() || null, + }; +} ``` -**Components (Pattern B, ~10 call sites):** +**Step 3: Delete `buildOpsMap`, `opsRegistry`, `ConnectionOps`.** + +These are now dead code. + +**Step 4: Update consumers.** + +The 15 files that call `connectionPool.get(projectId)` or `connectionPool.getActiveOps()` change from: + ```typescript -// Before: const ops = connectionPool.get(projectId); -const { updateChecklistAnswer, addPdfToStudy, addAnnotation } = ops; // all any +ops?.updateChecklist(studyId, checklistId, updates); +``` + +To: + +```typescript +const ops = connectionPool.getOps(projectId); +ops?.checklist.updateChecklist(studyId, checklistId, updates); +``` + +This is a mechanical find-and-replace. Every call site gains full IDE autocompletion and compile-time type checking. + +### Why not wrapper handles? + +The redesign RFC proposed `ChecklistHandle` classes that wrap the ops and bind `studyId`/`checklistId`. This is delegation, not abstraction. Problems: + +- Creates a new object per render (even with `useMemo`, identity is per-parent) +- Holds a reference to `entry.checklistOps` that can go stale on reconnection +- Two parents requesting the same `(studyId, checklistId)` get different object instances +- The "typed" `snapshot()` returns `Record` -- no real type improvement over what already exists + +If `studyId`/`checklistId` repetition is a problem in a specific component, a local destructured helper does the same job without a class or a hook. But in practice, editor components have `studyId` and `checklistId` in scope already. Passing them to each call is explicit and readable. + +--- + +## Fix 4: Remove Dead Indirection + +### The `project/actions/` layer + +The action modules in `project/actions/` follow this pattern: + +```typescript +export const checklistActions = { + update(studyId, checklistId, updates) { + const ops = connectionPool.getActiveOps(); + if (!ops) throw new Error('No active project connection'); + try { + ops.updateChecklist(studyId, checklistId, updates); + } catch (err) { + console.error('Error updating checklist:', err); + showToast.error('Update Failed', 'Failed to update checklist'); + } + }, +}; +``` -// After: -const ops = connectionPool.getTypedOps(projectId); -const { updateChecklistAnswer } = ops.checklist; -const { addPdfToStudy } = ops.pdf; -const { addAnnotation } = ops.annotation; +This adds one thing: a try/catch with a toast notification. It does not add domain logic. The call chain is: + +``` +Component + -> project.checklist.update(studyId, checklistId, updates) + -> checklistActions.update(...) + -> connectionPool.getActiveOps() // returns untyped flat map + -> ops.updateChecklist(...) // untyped wrapper lambda + -> entry.checklistOps.updateChecklist(...) // actual typed method + -> Y.Map.set() ``` -### Latent Bugs Discovered (surfaced by types) +That is four layers between the component and the Y.Doc mutation. After Fix 3 deletes `buildOpsMap`, it would be three. The thin action modules can be eliminated to make it two: + +``` +Component + -> connectionPool.getOps(projectId).checklist.updateChecklist(...) + -> Y.Map.set() +``` -1. **`getAwareness` destructured from ops but never registered** in `ReconciliationWrapper.tsx` -- always undefined, degrades silently -2. **`applyReconciliationToChecklists` called but not defined** on `ReconciliationOperations` -- dead code or missing implementation -3. **Signature mismatch** in reconciliation actions -- positional args forwarded as `any[]` hide parameter name misalignment +### Which action modules to delete -### Files to Create +| Module | Lines | Real Logic? | Verdict | +|--------|-------|-------------|---------| +| `checklists.ts` | 82 | No. Try/catch + toast only. | Delete. | +| `outcomes.ts` | ~40 | No. Same pattern. | Delete. | +| `reconciliation.ts` | ~30 | No. Same pattern. | Delete. | +| `members.ts` | ~30 | No. Same pattern. | Delete. | +| `project.ts` | ~30 | No. Forwards to studyOps rename/description. | Delete. | +| `pdfs.ts` | ~60 | Minor. Wraps upload + R2 delete. | Keep or inline. | +| `studies.ts` | 443 | Yes. Batch import, PDF upload, Google Drive import, metadata extraction. | Keep. Real domain logic. | -| File | Lines (est.) | -|---|---| -| `project/ProjectOps.ts` | ~15 | -| `project/errors.ts` | ~10 | +`studies.ts` is the only action module with logic worth keeping. The rest are forwarding functions. Their error handling (try/catch + toast) can either move to the component or to a thin shared utility if the pattern is common enough. -### Files to Modify (in order) +### The handle classes (in-progress work from RFC-A) -| File | Change | -|---|---| -| `project/ConnectionPool.ts` | Add `getTypedOps()`, `getActiveTypedOps()`, tighten ConnectionEntry field types | -| `project/actions/outcomes.ts` | 4 call sites | -| `project/actions/project.ts` | 2 call sites | -| `project/actions/reconciliation.ts` | 3 call sites + fix latent bugs | -| `project/actions/checklists.ts` | 7 call sites | -| `project/actions/pdfs.ts` | 6 call sites | -| `project/actions/studies.ts` | 5 call sites + helper function signatures | -| `components/project/completed-tab/CompletedTab.tsx` | 1 call site | -| `components/project/completed-tab/PreviousReviewersView.tsx` | 1 call site | -| `components/checklist/ChecklistYjsWrapper.tsx` | 1 call site | -| `components/project/reconcile-tab/ReconciliationWrapper.tsx` | 2 call sites + getAwareness fix | +The staged files on the current branch (`handles/ChecklistHandle.ts`, `handles/AnnotationHandle.ts`, `handles/PdfHandle.ts`, `handles/ReconciliationHandle.ts` and their corresponding hooks) are pure delegation wrappers that were part of the redesign RFC. They should not be merged. They add a layer without removing one. -### Cleanup (after full migration) +--- -Delete from `ConnectionPool.ts`: `buildOpsMap()`, `opsRegistry`, `get()`, `getActiveOps()`, `ConnectionOps` type. +## What Does Not Change + +| Component | Status | +|-----------|--------| +| `ConnectionPool.ts` | Stays. Loses `buildOpsMap` and `opsRegistry` (~60 lines). Gains `getOps()` returning typed interface (~15 lines). Net reduction. | +| `connectionReducer.ts` | Unchanged. Clean state machine. | +| `connection.ts` | Unchanged. WebSocket management. | +| `projectStore.ts` selectors | Unchanged. `selectStudies`, `selectStudy`, `selectMembers`, `selectMeta`, `selectConnectionPhase` all continue to work. | +| `projectStore.ts` shape | Unchanged. `projects` map, `connections`, `projectStats` all stay. | +| Component data access (reads) | Unchanged. Components still call `useProjectStore(s => selectStudies(s, projectId))`. | +| Operation factories | Unchanged. `createChecklistOperations`, etc. remain the logic layer. | +| Checklist type handlers | Unchanged. `AMSTAR2Handler`, `ROB2Handler`, `ROBINSIHandler` remain. | +| `extractAnswersFromYMap` | Stays in `sync.ts`. Called per-study now instead of for all studies, but the function itself does not change. | +| Y.Doc structure | Unchanged. Same maps, same nesting. | +| ProjectDoc (Durable Object) | Unchanged. Server-side improvements (alarm-based persistence, incremental updates) are orthogonal and can be done independently. | +| Zustand as the React integration | Stays. No `useSyncExternalStore`, no React context for session. Components subscribe to the same store with the same selectors. | --- -## Implementation Order +## Migration Plan -Parts A and B are independent and can be worked in parallel or sequentially. +Three phases, each independently shippable. Each phase is a single PR. No coexistence period, no dual-path reads. -**Recommended order:** +### Phase 1: Scoped Observation + Dirty Tracking (Fixes 1 and 2) -1. **Part B first** -- smaller scope (~15 files), mechanical migration, immediate type safety win, surfaces latent bugs. Can be done in 1-2 sessions. +**Goal**: Fix the performance problem. Zero component changes. -2. **Part A Phase 1** -- build ycollections library + Zustand bridge. Replace SyncManager. All existing consumers work unchanged. This is the high-impact change. +**Changes**: -3. **Part A Phase 2** -- migrate components from Zustand selectors to useSyncExternalStore hooks. Incremental, low-risk. +1. Refactor `createSyncManager` (in `sync.ts`): + - Add `attach(ydoc)` method that sets up scoped observers on `reviews`, `members`, `meta` + - Add per-study `Map` cache + - `observeDeep` on `reviews` extracts dirty study IDs from `event.path` + - `doSync()` only rebuilds dirty studies, passes unchanged snapshots by reference + - Return cleanup functions from `attach()` + - Add `pause()` / `resume()` for the `isLoadingPersistedState` guard -4. **Final cleanup** -- remove all old code, backward compat layers, and dead code. See below. +2. Update `ConnectionPool.initializeConnection`: + - Replace `ydoc.on('update', syncUpdateHandler)` with `syncManager.attach(ydoc)` + - Replace `isLoadingPersistedState` flag with `syncManager.pause()` / `syncManager.resume()` -## Branching Strategy +3. Update `projectStore.setProjectData`: + - Remove `JSON.stringify` equality checks for `studies`, `meta`, `members` + - Just set the values directly. With cached snapshots providing reference stability, Zustand's built-in `Object.is` selector comparison handles the rest at the component level. -All work happens on a feature branch (e.g., `feat/yjs-sync-redesign`). The branch is not merged until the full cleanup phase is complete. This means no backward compat shims survive into main -- the branch lands as a clean, finished refactor. +4. Initial sync: the first `doSync()` call populates the study cache from scratch (same as current behavior). Subsequent calls are incremental. -## Final Cleanup Phase (Step 4) +**Files changed**: `sync.ts` (bulk of changes), `ConnectionPool.ts` (~5 lines), `projectStore.ts` (~6 lines). -Everything below is deleted or removed before the branch merges. No backward compat layers ship to main. +**Validation**: +- `pnpm --filter landing test` (unit tests) +- `pnpm --filter landing test:browser` (E2E) +- Manual: open a 20+ study project, edit a checklist answer, verify via React DevTools that only the edited study's component subtree re-renders +- Profile: compare before/after for `doSync` execution time on a 50-study project -### Files to delete entirely: -- `primitives/useProject/sync.ts` -- replaced by ycollections -- `project/ProjectOps.ts` -- inline the type into ConnectionPool if preferred, or keep; but the point is `ConnectionOps` is gone +**Risk**: Low. The data flow to components is identical. The store receives the same `StudyInfo[]` shape. The only difference is that unchanged studies keep their object reference. -### Code to remove from `project/ConnectionPool.ts`: -- `buildOpsMap()` private method (60 lines) -- `opsRegistry` map and all references -- `get()` method (replaced by `getTypedOps()`) -- `getActiveOps()` method (replaced by `getActiveTypedOps()`) -- `ConnectionOps` type export -- `syncManager` field from ConnectionEntry (replaced by `collections`) -- `createSyncManager` import -- The `ydoc.on('update', syncUpdateHandler)` handler and its cleanup registration -- The `isLoadingPersistedState`-guarded sync call (collections handle this via pause/resume) +### Phase 2: Type the Operations Interface (Fix 3) -### Code to remove from `stores/projectStore.ts`: -- `JSON.stringify` equality checks in `setProjectData` (lines 131-149) -- After Phase 2 hook migration: remove `studies`, `members`, `meta` from `ProjectData` interface and `projects` state (store retains only connection state, project stats, pending data) -- Remove selectors that are fully replaced by collection hooks: `selectStudies`, `selectMembers`, `selectMeta`, `selectStudy`, `selectChecklist`, `selectStudyPdfs`, `selectPrimaryPdf`, `selectActiveProject`, `selectProject` -- Remove `EMPTY_STUDIES`, `EMPTY_MEMBERS`, `EMPTY_META`, `EMPTY_PDFS` fallback constants +**Goal**: Full type safety for all Y.Doc mutations. Mechanical refactor. -### Code to remove from `lib/ycollections/zustand-bridge.ts`: -- Delete the entire file once all components use `useSyncExternalStore` hooks directly +**Changes**: -### Imports to clean up across the codebase: -- Remove all `import { connectionPool } from '@/project/ConnectionPool'` calls that used `get()` or `getActiveOps()` -- replace with `getTypedOps()` / `getActiveTypedOps()` -- Remove all `import { createSyncManager } from '@/primitives/useProject/sync'` -- Remove all `import { selectStudies, selectMembers, ... } from '@/stores/projectStore'` in components migrated to hooks +1. Audit operation factories for exported interfaces: + - `ChecklistOperations` in `checklists/index.ts` -- exists + - `StudyOperations`, `PdfOperations`, `ReconciliationOperations`, `AnnotationOperations`, `OutcomeOperations` -- verify or create -### Dead code surfaced by typed ops migration: -- `applyReconciliationToChecklists` in `actions/reconciliation.ts` -- either implement or remove -- `getAwareness` destructuring in `ReconciliationWrapper.tsx` -- replace with `connectionPool.getAwareness(projectId)` -- Any reconciliation action signature mismatches -- fix or remove +2. Define `TypedProjectOps` in `ConnectionPool.ts`. -## Verification +3. Add `getOps(projectId): TypedProjectOps | null` method. -### Part B verification: -- `pnpm typecheck` -- zero type errors after each migration step -- `pnpm --filter landing test` -- existing unit tests pass -- `pnpm --filter landing test:browser` -- E2E tests pass (rob2-workflow, amstar2-workflow, robins-i-workflow) -- Manual: open a project, fill a checklist, reconcile -- operations work identically +4. Update all consumers of `connectionPool.get()` / `connectionPool.getActiveOps()`: + - `ops.updateChecklist(...)` becomes `ops.checklist.updateChecklist(...)` + - `ops.addPdfToStudy(...)` becomes `ops.pdf.addPdfToStudy(...)` + - Mechanical namespace change. -### Part A verification: -- New unit tests for YCollection (pure Y.Doc fixtures, no mocks) -- New unit tests for each extractor (extractStudy, extractROB2Answers, etc.) -- `pnpm --filter landing test:browser` -- E2E tests pass -- Manual: verify that editing a checklist in one tab updates the other tab's view -- Manual: verify that offline edits (via Dexie) sync correctly on reconnect -- Performance: verify that typing in a checklist comment does NOT trigger console logs for other study extractions (add temporary logging to verify incremental behavior) +5. Delete `buildOpsMap`, `opsRegistry`, `ConnectionOps` type, old `get()` method. -## Key Design Decisions +**Files changed**: `ConnectionPool.ts`, ~15 consumer files (action modules + components that access ops directly). -| Decision | Rationale | -|---|---| -| `observeDeep` on root maps, not per-entry `observe` | One observer per collection instead of N observers. Simpler lifecycle management. O(events) not O(entries). | -| Skip answer extraction for non-finalized checklists | Editors already use direct Y.Doc access. Extraction is wasted work. This is the single biggest performance win. | -| Zustand bridge for backward compat | 12+ selector consumers continue working without changes. Migration is incremental. | -| Namespaced ops (`ops.study.create`) over flat (`ops.createStudy`) | Prevents method collision, improves IDE navigability, matches the existing `project.study.create` action namespace | -| Throw on missing connection instead of returning null | Every consumer already throws. Centralizing it removes ~35 null-check guards. | -| One extractor file per checklist type | Eliminates 95% duplicated extraction logic. Each type is independently testable. Adding a new checklist type = one file. | +**Validation**: +- `pnpm typecheck` (primary gate -- type errors reveal any missed call site) +- `pnpm --filter landing test` +- `pnpm --filter landing test:browser` -## Reference Material +**Risk**: Low. Mechanical refactor. TypeScript catches every mistake at compile time. -Patterns adopted from reference libraries: -- **Zero Sync** (`reference/zero-mono`): `observeDeep` + incremental extraction mirrors their IVM operator pipeline -- **TanStack DB** (`reference/tanstack-db`): Per-collection independent sync, `useSyncExternalStore` hooks -- **Convex** (`reference/convex`): Shallow reference comparison, query-level change tracking -- **Outline** (`reference/outline`): Direct Y.Doc binding for editors, extraction only for persistence/display +### Phase 3: Remove Thin Action Wrappers (Fix 4) -## Current Architecture (for reference) +**Goal**: Reduce indirection. Optional but recommended. -### Files being replaced: -- `packages/landing/src/primitives/useProject/sync.ts` (505 lines) -- full tree extraction + Zustand push -- `packages/landing/src/project/ConnectionPool.ts` buildOpsMap (60 lines) -- untyped ops flattening +**Changes**: -### Y.Doc hierarchy: -``` -ydoc -+-- "reviews" Y.Map (studies, most volatile) -| +-- [studyId] Y.Map -| +-- name, description, metadata fields... -| +-- checklists Y.Map -| | +-- [checklistId] Y.Map (type, status, assignedTo, answers Y.Map) -| +-- pdfs Y.Map -| +-- annotations Y.Map -| +-- reconciliations Y.Map -+-- "meta" Y.Map (project metadata + outcomes, rarely changes) -+-- "members" Y.Map (member list, rarely changes) -``` +1. Delete thin action modules: `checklists.ts`, `outcomes.ts`, `reconciliation.ts`, `members.ts`, `project.ts`. + +2. Update components that called these actions to use `connectionPool.getOps()` directly. + +3. Keep `studies.ts` -- update it to use `TypedProjectOps`. + +4. If the try/catch + toast pattern repeats at 3+ call sites for the same operation, extract a utility: + ```typescript + function withToast(fn: () => T, errorTitle: string): T | null { + try { return fn(); } + catch (err) { + console.error(err); + showToast.error(errorTitle, (err as Error).message); + return null; + } + } + ``` + +5. Delete the staged handle files and handle hooks from the RFC-A work. -### Consumer patterns: -- ~12 components read from useProjectStore via selectors (read-only display) -- ~5 components access connectionPool.get() for direct Y.Doc operations (editors) -- ~35 action call sites use connectionPool.getActiveOps() for mutations -- Y.Text fields (checklist comments, preliminary fields) already bypass the store entirely +**Files changed**: 5 action modules deleted, ~10 consumer files updated. + +**Validation**: +- `pnpm typecheck` +- `pnpm --filter landing test` +- `pnpm --filter landing test:browser` +- `grep -r "checklistActions\|outcomeActions\|reconciliationActions\|memberActions\|projectActions" src/` returns zero results + +**Risk**: Low. No logic changes. Just removing forwarding functions. + +--- + +## Risk Assessment + +| Risk | Severity | Mitigation | +|------|----------|------------| +| `observeDeep` event path incorrect for some Y.Doc structure | Medium | Unit test: create Y.Doc, mutate a nested key, assert `event.path[0]` is the study ID. Y.js source confirms this behavior for nested Y.Maps. | +| Study cache gets stale (snapshot diverges from Y.Doc) | Medium | Cache is populated inside the `observeDeep` callback, which fires synchronously at end of Y.Doc transaction. No async gap. Also: `syncFromYDocImmediate()` can clear the cache and rebuild from scratch as a safety valve. | +| `requestAnimationFrame` batching drops events | Low | `observeDeep` fires with accumulated events per transaction. The RAF deduplication only affects the store write, not the event collection. Dirty study IDs accumulate across RAF frames via the study cache (which is updated synchronously in the callback). | +| Consumers missed during `get()` to `getOps()` migration | None | `pnpm typecheck` catches every call site. `get()` is deleted, so any unconverted call is a compile error. | +| Performance regression from `observeDeep` overhead | Low | `observeDeep` is how Y.js recommends observing nested changes. The overhead is lazy path computation, which is far cheaper than the current full-tree `toJSON()` + `JSON.stringify` comparison. Profile in Phase 1 validation. | + +--- + +## Comparison with Redesign RFC + +| | Redesign RFC (A) | Targeted Fixes (B) | +|---|---|---| +| Files significantly changed | 36+ across 6 phases | ~8 across 3 phases | +| Component API changes | 73 components migrate to new hooks | Zero for Phase 1. 15 files for Phase 2 (mechanical namespace change). | +| New abstractions introduced | Collections, Handles, Sessions, SessionManager, Providers, SessionContext | None | +| Abstractions removed | sync.ts, projectStore selectors, ConnectionPool, action modules | buildOpsMap, opsRegistry, ConnectionOps type, thin action modules | +| Coexistence period | Months (dual-path reads through Phases 1-3) | None. Each phase replaces in-place. | +| Performance improvement | O(1 study) per change | O(1 study) per change | +| Type safety improvement | Handles with `Record` snapshots | Typed ops with full `ChecklistOperations` etc. signatures | +| Risk of stalling mid-migration | High (Phase 3 is indirection until Phase 4 absorbs factories) | Low (each phase is independently complete) | +| Total lines added (estimated) | 1500+ (new abstractions) | ~200 (cache logic in sync.ts, typed interface) | +| Total lines removed (estimated) | 1200 (after full 6-phase migration) | ~400 (buildOpsMap, thin wrappers, JSON.stringify checks) | diff --git a/packages/landing/src/primitives/useProject/sync.ts b/packages/landing/src/primitives/useProject/sync.ts index 88785341b..01ae50604 100644 --- a/packages/landing/src/primitives/useProject/sync.ts +++ b/packages/landing/src/primitives/useProject/sync.ts @@ -106,27 +106,100 @@ interface MemberEntry { export interface SyncManager { syncFromYDoc: () => void; syncFromYDocImmediate: () => void; + attach: (ydoc: Y.Doc) => void; + detach: () => void; + pause: () => void; + resume: () => void; } export function createSyncManager(projectId: string, getYDoc: () => Y.Doc | null): SyncManager { let pendingSync = false; + let paused = false; + const cleanupHandlers: (() => void)[] = []; - function doSync(): void { + // Per-slice dirty flags -- set by scoped observers, consumed by doSync + const dirtySlices = { studies: false, members: false, meta: false }; + + // Per-study snapshot cache -- dirty studies get rebuilt, clean studies keep their reference + const studyCache = new Map(); + let sortedStudies: StudyInfo[] = []; + + function handleReviewsEvents(events: Y.YEvent[]): void { + if (paused) return; + + const reviewsMap = getYDoc()?.getMap('reviews'); + if (!reviewsMap) return; + + const dirtyStudyIds = new Set(); + let structuralChange = false; + + for (const event of events) { + if (event.path.length === 0 && 'keys' in event) { + // Top-level: studies added or removed from the reviews map + structuralChange = true; + for (const [key] of (event as Y.YMapEvent).keys) { + dirtyStudyIds.add(key); + } + } else if (event.path.length > 0) { + // Nested: something inside a specific study changed + dirtyStudyIds.add(String(event.path[0])); + } + } + + // Rebuild only dirty studies + for (const studyId of dirtyStudyIds) { + const studyYMap = reviewsMap.get(studyId) as Y.Map | undefined; + if (studyYMap) { + const studyData = studyYMap.toJSON ? studyYMap.toJSON() : {}; + studyCache.set( + studyId, + buildStudyFromYMap(studyId, studyData as Record, studyYMap), + ); + } else { + studyCache.delete(studyId); + } + } + + // Clean up entries for studies that no longer exist in the Y.Map + if (structuralChange) { + for (const cachedId of studyCache.keys()) { + if (!reviewsMap.has(cachedId)) { + studyCache.delete(cachedId); + } + } + } + + if (dirtyStudyIds.size > 0 || structuralChange) { + sortedStudies = [...studyCache.values()].sort( + (a, b) => (a.createdAt || 0) - (b.createdAt || 0), + ); + dirtySlices.studies = true; + scheduleSync(); + } + } + + function rebuildAllStudies(): void { const ydoc = getYDoc(); if (!ydoc) return; - const studiesMap = ydoc.getMap('reviews'); - const studiesList: StudyInfo[] = []; + const reviewsMap = ydoc.getMap('reviews'); + studyCache.clear(); - for (const [studyId, studyYMap] of studiesMap.entries()) { + for (const [studyId, studyYMap] of reviewsMap.entries()) { const ymap = studyYMap as Y.Map; - const studyData = ymap.toJSON ? ymap.toJSON() : (studyYMap as Record); - const study = buildStudyFromYMap(studyId, studyData as Record, ymap); - studiesList.push(study); + const studyData = ymap.toJSON ? ymap.toJSON() : {}; + studyCache.set( + studyId, + buildStudyFromYMap(studyId, studyData as Record, ymap), + ); } - studiesList.sort((a, b) => (a.createdAt || 0) - (b.createdAt || 0)); + sortedStudies = [...studyCache.values()].sort( + (a, b) => (a.createdAt || 0) - (b.createdAt || 0), + ); + } + function buildMetaData(ydoc: Y.Doc): Record { const metaMap = ydoc.getMap('meta'); const metaData = metaMap.toJSON ? metaMap.toJSON() : ({} as Record); @@ -138,23 +211,50 @@ export function createSyncManager(projectId: string, getYDoc: () => Y.Doc | null const outcomeData = ymap.toJSON ? ymap.toJSON() : (outcomeYMap as Record); outcomesList.push({ id: outcomeId, ...outcomeData }); } - outcomesList.sort((a, b) => ((a.createdAt as number) || 0) - ((b.createdAt as number) || 0)); + outcomesList.sort( + (a, b) => ((a.createdAt as number) || 0) - ((b.createdAt as number) || 0), + ); metaData.outcomes = outcomesList; } else { metaData.outcomes = []; } - const membersMap = ydoc.getMap('members'); - const membersList = buildMembersList(membersMap); + return metaData; + } - useProjectStore.getState().setProjectData(projectId, { - studies: studiesList as any, - meta: metaData, - members: membersList, - }); + function doSync(): void { + const ydoc = getYDoc(); + if (!ydoc) return; + + const updates: { studies?: StudyInfo[]; meta?: Record; members?: MemberEntry[] } = {}; + + if (dirtySlices.studies) { + // If the cache is empty, this is the first sync -- populate from scratch + if (studyCache.size === 0) { + rebuildAllStudies(); + } + updates.studies = sortedStudies; + } + + if (dirtySlices.members) { + updates.members = buildMembersList(ydoc.getMap('members')); + } + + if (dirtySlices.meta) { + updates.meta = buildMetaData(ydoc); + } + + // Reset dirty flags before store update + dirtySlices.studies = false; + dirtySlices.members = false; + dirtySlices.meta = false; + + if (updates.studies !== undefined || updates.members !== undefined || updates.meta !== undefined) { + useProjectStore.getState().setProjectData(projectId, updates as any); + } } - function syncFromYDoc(): void { + function scheduleSync(): void { if (pendingSync) return; pendingSync = true; requestAnimationFrame(() => { @@ -163,12 +263,78 @@ export function createSyncManager(projectId: string, getYDoc: () => Y.Doc | null }); } + function syncFromYDoc(): void { + if (paused) return; + // Mark all slices dirty (used by legacy callers and Dexie write-back) + dirtySlices.studies = true; + dirtySlices.members = true; + dirtySlices.meta = true; + scheduleSync(); + } + function syncFromYDocImmediate(): void { pendingSync = false; + // Full rebuild: clear cache so doSync repopulates from scratch + studyCache.clear(); + sortedStudies = []; + dirtySlices.studies = true; + dirtySlices.members = true; + dirtySlices.meta = true; doSync(); } - return { syncFromYDoc, syncFromYDocImmediate }; + function attach(ydoc: Y.Doc): void { + const reviewsMap = ydoc.getMap('reviews'); + const membersMap = ydoc.getMap('members'); + const metaMap = ydoc.getMap('meta'); + + const onReviews = (events: Y.YEvent[]) => { + handleReviewsEvents(events); + }; + const onMembers = () => { + if (paused) return; + dirtySlices.members = true; + scheduleSync(); + }; + const onMeta = () => { + if (paused) return; + dirtySlices.meta = true; + scheduleSync(); + }; + + reviewsMap.observeDeep(onReviews); + membersMap.observe(onMembers); + metaMap.observeDeep(onMeta); + + cleanupHandlers.push( + () => reviewsMap.unobserveDeep(onReviews), + () => membersMap.unobserve(onMembers), + () => metaMap.unobserveDeep(onMeta), + ); + } + + function detach(): void { + for (const cleanup of cleanupHandlers) { + try { + cleanup(); + } catch (_) { + /* ignore */ + } + } + cleanupHandlers.length = 0; + studyCache.clear(); + sortedStudies = []; + } + + function pause(): void { + paused = true; + } + + function resume(): void { + paused = false; + } + + return { syncFromYDoc, syncFromYDocImmediate, attach, detach, pause, resume }; } function buildStudyFromYMap( diff --git a/packages/landing/src/project/ConnectionPool.ts b/packages/landing/src/project/ConnectionPool.ts index 88517ef09..d3731bcad 100644 --- a/packages/landing/src/project/ConnectionPool.ts +++ b/packages/landing/src/project/ConnectionPool.ts @@ -35,7 +35,6 @@ export interface ConnectionEntry { outcomeOps: any; refCount: number; initialized: boolean; - isLoadingPersistedState: boolean; _cleanupHandlers: (() => void)[]; } @@ -74,7 +73,6 @@ class ConnectionPool { outcomeOps: null, refCount: 1, initialized: false, - isLoadingPersistedState: false, _cleanupHandlers: [], }; @@ -115,14 +113,9 @@ class ConnectionPool { // Build flat operations map this.opsRegistry.set(projectId, this.buildOpsMap(entry)); - // YDoc update handler (debounced sync to Zustand store) - const syncUpdateHandler = () => { - if (!entry.isLoadingPersistedState) { - entry.syncManager?.syncFromYDoc(); - } - }; - ydoc.on('update', syncUpdateHandler); - entry._cleanupHandlers.push(() => ydoc.off('update', syncUpdateHandler)); + // Scoped Y.Map observers (reviews, members, meta) for incremental sync + entry.syncManager.attach(ydoc); + entry._cleanupHandlers.push(() => entry.syncManager?.detach()); // Dexie persistence (async) (db.projects as any).get(projectId).then(async (existingProject: any) => { @@ -141,7 +134,7 @@ class ConnectionPool { entry.dexieProvider.whenLoaded.then(() => { if (cancelled()) return; - entry.isLoadingPersistedState = true; + entry.syncManager?.pause(); try { const persistedState = Y.encodeStateAsUpdate(project.ydoc); Y.applyUpdate(ydoc, persistedState); @@ -149,7 +142,7 @@ class ConnectionPool { console.error('Corrupted persisted state, clearing local data:', err); deleteProjectData(projectId).catch(() => {}); } finally { - entry.isLoadingPersistedState = false; + entry.syncManager?.resume(); } // Dexie write-back handler diff --git a/packages/landing/src/stores/projectStore.ts b/packages/landing/src/stores/projectStore.ts index a0fbdbe7d..9e4ad01e6 100644 --- a/packages/landing/src/stores/projectStore.ts +++ b/packages/landing/src/stores/projectStore.ts @@ -128,25 +128,19 @@ export const useProjectStore = create() } const project = state.projects[projectId]; if (data.meta !== undefined) { - if (JSON.stringify(project.meta) !== JSON.stringify(data.meta)) { - project.meta = data.meta; - } + project.meta = data.meta; } if (data.members !== undefined) { - if (JSON.stringify(project.members) !== JSON.stringify(data.members)) { - project.members = data.members; - } + project.members = data.members; } if (data.studies !== undefined) { - if (JSON.stringify(project.studies) !== JSON.stringify(data.studies)) { - project.studies = data.studies; - const stats = computeProjectStats(data.studies); - state.projectStats[projectId] = { - ...stats, - lastUpdated: Date.now(), - }; - studiesChanged = true; - } + project.studies = data.studies; + const stats = computeProjectStats(data.studies); + state.projectStats[projectId] = { + ...stats, + lastUpdated: Date.now(), + }; + studiesChanged = true; } }); if (studiesChanged) { From cae52cb2bdbc737ef5104ea7b372d12d72222696 Mon Sep 17 00:00:00 2001 From: Jacob Maynard Date: Sat, 28 Mar 2026 21:29:55 -0500 Subject: [PATCH 2/7] phase 2 --- .../checklist/ChecklistYjsWrapper.tsx | 18 ++- .../project/completed-tab/CompletedTab.tsx | 4 +- .../completed-tab/PreviousReviewersView.tsx | 10 +- .../reconcile-tab/ReconciliationWrapper.tsx | 27 ++-- .../landing/src/project/ConnectionPool.ts | 140 +++++++----------- .../landing/src/project/actions/checklists.ts | 16 +- .../landing/src/project/actions/outcomes.ts | 8 +- packages/landing/src/project/actions/pdfs.ts | 46 +++--- .../landing/src/project/actions/project.ts | 4 +- .../src/project/actions/reconciliation.ts | 11 +- .../landing/src/project/actions/studies.ts | 60 ++++---- 11 files changed, 158 insertions(+), 186 deletions(-) diff --git a/packages/landing/src/components/checklist/ChecklistYjsWrapper.tsx b/packages/landing/src/components/checklist/ChecklistYjsWrapper.tsx index 2df9f5e7a..1473950c3 100644 --- a/packages/landing/src/components/checklist/ChecklistYjsWrapper.tsx +++ b/packages/landing/src/components/checklist/ChecklistYjsWrapper.tsx @@ -80,20 +80,22 @@ export function ChecklistYjsWrapper({ projectId, studyId, checklistId }: Checkli const [selectedPdfId, setSelectedPdfId] = useState(null); const [attemptedPdfFile, setAttemptedPdfFile] = useState(null); - const ops = connectionPool.get(projectId); + const ops = connectionPool.getOps(projectId); if (!ops) throw new Error(`No connection for project ${projectId}`); const { updateChecklistAnswer, updateChecklist, getChecklistData, - addPdfToStudy, getQuestionNote, getRobinsText, getRob2Text, + } = ops.checklist; + const { addPdfToStudy } = ops.pdf; + const { addAnnotation, updateAnnotation, deleteAnnotation, - } = ops; + } = ops.annotation; const connectionState = useProjectStore(s => selectConnectionPhase(s, projectId)); @@ -198,7 +200,7 @@ export function ChecklistYjsWrapper({ projectId, studyId, checklistId }: Checkli key: uploadResult.key, fileName: uploadResult.fileName, size: uploadResult.size, - uploadedBy: user?.id, + uploadedBy: user?.id ?? '', uploadedAt: Date.now(), }, tag, @@ -232,7 +234,7 @@ export function ChecklistYjsWrapper({ projectId, studyId, checklistId }: Checkli name: currentStudy?.name || 'Checklist', reviewerName: '', createdAt: currentChecklist.createdAt, - ...data.answers, + ...(data.answers as Record ?? {}), }; }, [currentChecklist, currentStudy, getChecklistData, studyId, checklistId]); @@ -249,9 +251,9 @@ export function ChecklistYjsWrapper({ projectId, studyId, checklistId }: Checkli const isChecklistValid = useMemo(() => { if (!checklistForUI) return false; - if (checklistType === 'AMSTAR2') return isAMSTAR2Complete(checklistForUI); - if (checklistType === 'ROBINS_I') return isROBINSIComplete(checklistForUI); - if (checklistType === 'ROB2') return isROB2Complete(checklistForUI); + if (checklistType === 'AMSTAR2') return isAMSTAR2Complete(checklistForUI as any); + if (checklistType === 'ROBINS_I') return isROBINSIComplete(checklistForUI as any); + if (checklistType === 'ROB2') return isROB2Complete(checklistForUI as any); return true; }, [checklistForUI, checklistType]); diff --git a/packages/landing/src/components/project/completed-tab/CompletedTab.tsx b/packages/landing/src/components/project/completed-tab/CompletedTab.tsx index ee27c0d84..517050cd6 100644 --- a/packages/landing/src/components/project/completed-tab/CompletedTab.tsx +++ b/packages/landing/src/components/project/completed-tab/CompletedTab.tsx @@ -15,9 +15,9 @@ import { project } from '@/project'; export function CompletedTab() { const { projectId, getAssigneeName, getChecklistPath } = useProjectContext(); const navigate = useNavigate(); - const conn = connectionPool.get(projectId); + const conn = connectionPool.getOps(projectId); if (!conn) throw new Error(`No connection for project ${projectId}`); - const getAllReconciliationProgress = conn.getAllReconciliationProgress; + const getAllReconciliationProgress = conn.reconciliation.getAllReconciliationProgress; const studies = useProjectStore(s => selectStudies(s, projectId)); const meta = useProjectStore(s => s.projects[projectId]?.meta) as any; diff --git a/packages/landing/src/components/project/completed-tab/PreviousReviewersView.tsx b/packages/landing/src/components/project/completed-tab/PreviousReviewersView.tsx index c2cc1b6eb..4f370899f 100644 --- a/packages/landing/src/components/project/completed-tab/PreviousReviewersView.tsx +++ b/packages/landing/src/components/project/completed-tab/PreviousReviewersView.tsx @@ -34,10 +34,10 @@ export function PreviousReviewersView({ onClose, }: PreviousReviewersViewProps) { const { projectId } = useProjectContext(); - const ops = connectionPool.get(projectId); + const ops = connectionPool.getOps(projectId); if (!ops) throw new Error(`No connection for project ${projectId}`); - const getChecklistData = ops.getChecklistData; - const getQuestionNote = ops.getQuestionNote; + const getChecklistData = ops.checklist.getChecklistData; + const getQuestionNote = ops.checklist.getQuestionNote; const [checklist1Data, setChecklist1Data] = useState(null); const [checklist2Data, setChecklist2Data] = useState(null); @@ -75,7 +75,7 @@ export function PreviousReviewersView({ reviewerName: getAssigneeName(String(originalChecklists[0].assignedTo ?? '')), createdAt: originalChecklists[0].createdAt, type: originalChecklists[0].type, - ...data1.answers, + ...(data1.answers as Record ?? {}), } : null, ); @@ -88,7 +88,7 @@ export function PreviousReviewersView({ reviewerName: getAssigneeName(String(originalChecklists[1].assignedTo ?? '')), createdAt: originalChecklists[1].createdAt, type: originalChecklists[1].type, - ...data2.answers, + ...(data2.answers as Record ?? {}), } : null, ); diff --git a/packages/landing/src/components/project/reconcile-tab/ReconciliationWrapper.tsx b/packages/landing/src/components/project/reconcile-tab/ReconciliationWrapper.tsx index 68bae2c3a..7ded39534 100644 --- a/packages/landing/src/components/project/reconcile-tab/ReconciliationWrapper.tsx +++ b/packages/landing/src/components/project/reconcile-tab/ReconciliationWrapper.tsx @@ -60,21 +60,22 @@ export function ReconciliationWrapper({ closePreview(); }, []); // eslint-disable-line react-hooks/exhaustive-deps - // Destructure Y.js operations from connection pool - const ops = connectionPool.get(projectId); + const ops = connectionPool.getOps(projectId); if (!ops) throw new Error(`No connection for project ${projectId}`); const { createChecklist: createProjectChecklist, updateChecklistAnswer, updateChecklist, getChecklistData, - getReconciliationProgress, getQuestionNote, getRobinsText, getRob2Text, + } = ops.checklist; + const { + getReconciliationProgress, saveReconciliationProgress, - getAwareness, - } = ops; + } = ops.reconciliation; + const getAwareness = ops.getAwareness; // Current user for presence features const currentUser = useMemo(() => { @@ -207,7 +208,7 @@ export function ReconciliationWrapper({ name: currentStudy?.name || 'Checklist 1', reviewerName: getReviewerName(checklist1Meta.assignedTo), createdAt: checklist1Meta.createdAt, - ...data.answers, + ...(data.answers as Record ?? {}), }; }, [ checklist1Meta, @@ -227,7 +228,7 @@ export function ReconciliationWrapper({ name: currentStudy?.name || 'Checklist 2', reviewerName: getReviewerName(checklist2Meta.assignedTo), createdAt: checklist2Meta.createdAt, - ...data.answers, + ...(data.answers as Record ?? {}), }; }, [ checklist2Meta, @@ -388,7 +389,7 @@ export function ReconciliationWrapper({ name: 'Reconciled Checklist', reviewerName: 'Consensus', createdAt: reconciledChecklistMeta?.createdAt || 0, - ...data.answers, + ...(data.answers as Record ?? {}), }; }, [reconciledChecklistId, getChecklistData, studyId, reconciledChecklistMeta]); @@ -427,13 +428,13 @@ export function ReconciliationWrapper({ const getTextRef = useCallback( (...args: unknown[]) => { if (isRobinsI) { - return getRobinsText(studyId, reconciledChecklistId, ...args); + return getRobinsText(studyId, reconciledChecklistId as string, ...(args as [string, string, string?])); } if (isRob2) { - return getRob2Text(studyId, reconciledChecklistId, ...args); + return getRob2Text(studyId, reconciledChecklistId as string, ...(args as [string, string, string?])); } // AMSTAR2: getQuestionNote takes just the question key - return getQuestionNote(studyId, reconciledChecklistId, args[0]); + return getQuestionNote(studyId, reconciledChecklistId as string, args[0] as string); }, [ isRobinsI, @@ -450,9 +451,9 @@ export function ReconciliationWrapper({ const setTextValue = useCallback( (params: { sectionKey?: string; fieldKey?: string; questionKey?: string }, text: string) => { if (!reconciledChecklistId) return; - const poolOps = connectionPool.get(projectId); + const poolOps = connectionPool.getOps(projectId); if (!poolOps) throw new Error(`No connection for project ${projectId}`); - poolOps.setTextValue(studyId, reconciledChecklistId, params, text); + poolOps.checklist.setTextValue(studyId, reconciledChecklistId, params, text); }, [studyId, reconciledChecklistId, projectId], ); diff --git a/packages/landing/src/project/ConnectionPool.ts b/packages/landing/src/project/ConnectionPool.ts index d3731bcad..55b9cd018 100644 --- a/packages/landing/src/project/ConnectionPool.ts +++ b/packages/landing/src/project/ConnectionPool.ts @@ -11,39 +11,57 @@ import { queryClient } from '@/lib/queryClient'; import { queryKeys } from '@/lib/queryKeys'; import { createConnectionManager } from '@/primitives/useProject/connection'; import { createSyncManager, type SyncManager } from '@/primitives/useProject/sync'; -import { createStudyOperations } from '@/primitives/useProject/studies'; +import { + createStudyOperations, + type StudyOperations, +} from '@/primitives/useProject/studies'; import { createChecklistOperations, type ChecklistOperations, } from '@/primitives/useProject/checklists/index'; -import { createPdfOperations } from '@/primitives/useProject/pdfs'; -import { createReconciliationOperations } from '@/primitives/useProject/reconciliation.js'; -import { createAnnotationOperations } from '@/primitives/useProject/annotations'; -import { createOutcomeOperations } from '@/primitives/useProject/outcomes.js'; +import { createPdfOperations, type PdfOperations } from '@/primitives/useProject/pdfs'; +import { + createReconciliationOperations, + type ReconciliationOperations, +} from '@/primitives/useProject/reconciliation.js'; +import { + createAnnotationOperations, + type AnnotationOperations, +} from '@/primitives/useProject/annotations'; +import { + createOutcomeOperations, + type OutcomeOperations, +} from '@/primitives/useProject/outcomes.js'; import { db, deleteProjectData } from '@/primitives/db.js'; +export interface TypedProjectOps { + study: StudyOperations; + checklist: ChecklistOperations; + pdf: PdfOperations; + reconciliation: ReconciliationOperations; + annotation: AnnotationOperations; + outcome: OutcomeOperations; + getAwareness: () => unknown; +} + export interface ConnectionEntry { ydoc: Y.Doc; dexieProvider: any; connectionManager: any; syncManager: SyncManager | null; - studyOps: any; + studyOps: StudyOperations | null; checklistOps: ChecklistOperations | null; - pdfOps: any; - reconciliationOps: any; - annotationOps: any; - outcomeOps: any; + pdfOps: PdfOperations | null; + reconciliationOps: ReconciliationOperations | null; + annotationOps: AnnotationOperations | null; + outcomeOps: OutcomeOperations | null; refCount: number; initialized: boolean; _cleanupHandlers: (() => void)[]; } -/** Flat operations object registered for a connection */ -export type ConnectionOps = Record any>; - class ConnectionPool { private registry = new Map(); - private opsRegistry = new Map(); private _activeProjectId: string | null = null; private _activeOrgId: string | null = null; @@ -110,9 +128,6 @@ class ConnectionPool { entry.annotationOps = createAnnotationOperations(projectId, getYDoc, isSynced); entry.outcomeOps = createOutcomeOperations(projectId, getYDoc, isSynced); - // Build flat operations map - this.opsRegistry.set(projectId, this.buildOpsMap(entry)); - // Scoped Y.Map observers (reviews, members, meta) for incremental sync entry.syncManager.attach(ydoc); entry._cleanupHandlers.push(() => entry.syncManager?.detach()); @@ -193,25 +208,38 @@ class ConnectionPool { } /** - * Get the flat operations map for a project connection. + * Get typed operations for a project connection. */ - get(projectId: string): ConnectionOps | null { - return this.opsRegistry.get(projectId) || null; + getOps(projectId: string): TypedProjectOps | null { + const entry = this.registry.get(projectId); + if (!entry?.initialized || !entry.studyOps || !entry.checklistOps || !entry.pdfOps || + !entry.reconciliationOps || !entry.annotationOps || !entry.outcomeOps) { + return null; + } + return { + study: entry.studyOps, + checklist: entry.checklistOps, + pdf: entry.pdfOps, + reconciliation: entry.reconciliationOps, + annotation: entry.annotationOps, + outcome: entry.outcomeOps, + getAwareness: () => entry.connectionManager?.getAwareness() || null, + }; } /** - * Get the raw ConnectionEntry for a project (for direct ops access). + * Get the raw ConnectionEntry for a project (for direct entry access). */ getEntry(projectId: string): ConnectionEntry | null { return this.registry.get(projectId) || null; } /** - * Get operations for the currently active project. + * Get typed operations for the currently active project. */ - getActiveOps(): ConnectionOps | null { + getActiveOps(): TypedProjectOps | null { if (!this._activeProjectId) return null; - return this.get(this._activeProjectId); + return this.getOps(this._activeProjectId); } /** @@ -296,72 +324,8 @@ class ConnectionPool { if (entry.ydoc) entry.ydoc.destroy(); this.registry.delete(projectId); - this.opsRegistry.delete(projectId); useProjectStore.getState().dispatchConnectionEvent(projectId, { type: 'RESET' }); } - - private buildOpsMap(entry: ConnectionEntry): ConnectionOps { - // Cast typed ops to any for the flat map -- callers already treat these as untyped - const chk = entry.checklistOps as any; - return { - // Study - createStudy: (...args: any[]) => entry.studyOps?.createStudy(...args), - updateStudy: (...args: any[]) => entry.studyOps?.updateStudy(...args), - deleteStudy: (...args: any[]) => entry.studyOps?.deleteStudy(...args), - renameProject: (...args: any[]) => entry.studyOps?.renameProject(...args), - updateDescription: (...args: any[]) => entry.studyOps?.updateDescription(...args), - updateProjectSettings: (...args: any[]) => entry.studyOps?.updateProjectSettings(...args), - // Checklist - createChecklist: (...args: any[]) => chk?.createChecklist(...args), - updateChecklist: (...args: any[]) => chk?.updateChecklist(...args), - deleteChecklist: (...args: any[]) => chk?.deleteChecklist(...args), - getChecklistAnswersMap: (...args: any[]) => chk?.getChecklistAnswersMap(...args), - getChecklistData: (...args: any[]) => chk?.getChecklistData(...args), - updateChecklistAnswer: (...args: any[]) => chk?.updateChecklistAnswer(...args), - getQuestionNote: (...args: any[]) => chk?.getQuestionNote(...args), - getRobinsText: (...args: any[]) => chk?.getRobinsText(...args), - getRob2Text: (...args: any[]) => chk?.getRob2Text(...args), - getTextRef: (...args: any[]) => chk?.getTextRef(...args), - setTextValue: (...args: any[]) => chk?.setTextValue(...args), - // PDF - addPdfToStudy: (...args: any[]) => entry.pdfOps?.addPdfToStudy(...args), - removePdfFromStudy: (...args: any[]) => entry.pdfOps?.removePdfFromStudy(...args), - removePdfByFileName: (...args: any[]) => entry.pdfOps?.removePdfByFileName(...args), - updatePdfTag: (...args: any[]) => entry.pdfOps?.updatePdfTag(...args), - updatePdfMetadata: (...args: any[]) => entry.pdfOps?.updatePdfMetadata(...args), - setPdfAsPrimary: (...args: any[]) => entry.pdfOps?.setPdfAsPrimary(...args), - setPdfAsProtocol: (...args: any[]) => entry.pdfOps?.setPdfAsProtocol(...args), - // Reconciliation - saveReconciliationProgress: (...args: any[]) => - entry.reconciliationOps?.saveReconciliationProgress(...args), - getReconciliationProgress: (...args: any[]) => - entry.reconciliationOps?.getReconciliationProgress(...args), - getAllReconciliationProgress: (...args: any[]) => - entry.reconciliationOps?.getAllReconciliationProgress(...args), - clearReconciliationProgress: (...args: any[]) => - entry.reconciliationOps?.clearReconciliationProgress(...args), - // Annotations - addAnnotation: (...args: any[]) => entry.annotationOps?.addAnnotation(...args), - addAnnotations: (...args: any[]) => entry.annotationOps?.addAnnotations(...args), - updateAnnotation: (...args: any[]) => entry.annotationOps?.updateAnnotation(...args), - deleteAnnotation: (...args: any[]) => entry.annotationOps?.deleteAnnotation(...args), - getAnnotations: (...args: any[]) => entry.annotationOps?.getAnnotations(...args), - getAllAnnotationsForPdf: (...args: any[]) => - entry.annotationOps?.getAllAnnotationsForPdf(...args), - clearAnnotationsForChecklist: (...args: any[]) => - entry.annotationOps?.clearAnnotationsForChecklist(...args), - mergeAnnotations: (...args: any[]) => entry.annotationOps?.mergeAnnotations(...args), - // Outcomes - getOutcomes: () => entry.outcomeOps?.getOutcomes() || [], - getOutcome: (...args: any[]) => entry.outcomeOps?.getOutcome(...args), - createOutcome: (...args: any[]) => entry.outcomeOps?.createOutcome(...args), - updateOutcome: (...args: any[]) => entry.outcomeOps?.updateOutcome(...args), - deleteOutcome: (...args: any[]) => entry.outcomeOps?.deleteOutcome(...args), - isOutcomeInUse: (...args: any[]) => entry.outcomeOps?.isOutcomeInUse(...args), - // Presence - getAwareness: () => entry.connectionManager?.getAwareness() || null, - }; - } } export const connectionPool = new ConnectionPool(); diff --git a/packages/landing/src/project/actions/checklists.ts b/packages/landing/src/project/actions/checklists.ts index 7ed6b4a94..1a5b0d077 100644 --- a/packages/landing/src/project/actions/checklists.ts +++ b/packages/landing/src/project/actions/checklists.ts @@ -10,7 +10,7 @@ export const checklistActions = { const ops = connectionPool.getActiveOps(); if (!ops) throw new Error('No active project connection'); try { - const checklistId = ops.createChecklist(studyId, type, assigneeId, outcomeId ?? null); + const checklistId = ops.checklist.createChecklist(studyId, type, assigneeId, outcomeId ?? null); if (!checklistId) { const requiresOutcome = type === 'ROB2' || type === 'ROBINS_I'; if (requiresOutcome && !outcomeId) { @@ -37,7 +37,7 @@ export const checklistActions = { const ops = connectionPool.getActiveOps(); if (!ops) throw new Error('No active project connection'); try { - ops.updateChecklist(studyId, checklistId, updates); + ops.checklist.updateChecklist(studyId, checklistId, updates); } catch (err) { console.error('Error updating checklist:', err); showToast.error('Update Failed', 'Failed to update checklist'); @@ -48,7 +48,7 @@ export const checklistActions = { const ops = connectionPool.getActiveOps(); if (!ops) throw new Error('No active project connection'); try { - ops.deleteChecklist(studyId, checklistId); + ops.checklist.deleteChecklist(studyId, checklistId); } catch (err) { console.error('Error deleting checklist:', err); showToast.error('Delete Failed', 'Failed to delete checklist'); @@ -58,24 +58,26 @@ export const checklistActions = { getAnswersMap(studyId: string, checklistId: string): unknown { const ops = connectionPool.getActiveOps(); if (!ops) throw new Error('No active project connection'); - return ops.getChecklistAnswersMap(studyId, checklistId); + return ops.checklist.getChecklistAnswersMap(studyId, checklistId); }, getData(studyId: string, checklistId: string): Record | null { const ops = connectionPool.getActiveOps(); if (!ops) throw new Error('No active project connection'); - return ops.getChecklistData(studyId, checklistId) ?? null; + return ops.checklist.getChecklistData(studyId, checklistId) ?? null; }, updateAnswer(studyId: string, checklistId: string, questionId: string, answer: unknown, note?: string): void { const ops = connectionPool.getActiveOps(); if (!ops) throw new Error('No active project connection'); - ops.updateChecklistAnswer(studyId, checklistId, questionId, answer, note); + const data: Record = { answer }; + if (note !== undefined) data.note = note; + ops.checklist.updateChecklistAnswer(studyId, checklistId, questionId, data); }, getQuestionNote(studyId: string, checklistId: string, questionId: string): unknown { const ops = connectionPool.getActiveOps(); if (!ops) throw new Error('No active project connection'); - return ops.getQuestionNote(studyId, checklistId, questionId); + return ops.checklist.getQuestionNote(studyId, checklistId, questionId); }, }; diff --git a/packages/landing/src/project/actions/outcomes.ts b/packages/landing/src/project/actions/outcomes.ts index 37e883754..4f0961523 100644 --- a/packages/landing/src/project/actions/outcomes.ts +++ b/packages/landing/src/project/actions/outcomes.ts @@ -14,24 +14,24 @@ export const outcomeActions = { console.error('[outcome.create] No user logged in'); return null; } - return conn.createOutcome(name, user.id); + return conn.outcome.createOutcome(name, user.id); }, update(outcomeId: string, name: string): boolean { const conn = connectionPool.getActiveOps(); if (!conn) throw new Error('No active project connection'); - return conn.updateOutcome(outcomeId, name); + return conn.outcome.updateOutcome(outcomeId, name); }, delete(outcomeId: string): { success: boolean; error?: string } { const conn = connectionPool.getActiveOps(); if (!conn) throw new Error('No active project connection'); - return conn.deleteOutcome(outcomeId); + return conn.outcome.deleteOutcome(outcomeId); }, isInUse(outcomeId: string): boolean { const conn = connectionPool.getActiveOps(); if (!conn) throw new Error('No active project connection'); - return conn.isOutcomeInUse(outcomeId); + return conn.outcome.isOutcomeInUse(outcomeId); }, }; diff --git a/packages/landing/src/project/actions/pdfs.ts b/packages/landing/src/project/actions/pdfs.ts index 449371f1d..d29c4a596 100644 --- a/packages/landing/src/project/actions/pdfs.ts +++ b/packages/landing/src/project/actions/pdfs.ts @@ -11,6 +11,7 @@ import { useProjectStore } from '@/stores/projectStore'; import { usePdfPreviewStore } from '@/stores/pdfPreviewStore'; import { useAuthStore, selectUser } from '@/stores/authStore'; import { connectionPool } from '../ConnectionPool'; +import type { PdfInfo, PdfTag } from '@/primitives/useProject/pdfs'; async function extractPdfMetadata( arrayBuffer: ArrayBuffer | null, @@ -147,24 +148,24 @@ export const pdfActions = { const pdfMetadata = await extractPdfMetadata(arrayBuffer); - const pdfId = ops.addPdfToStudy( + const pdfId = ops.pdf.addPdfToStudy( studyId, { key: uploadResult.key, fileName: uploadResult.fileName, size: uploadResult.size, - uploadedBy: userId, + uploadedBy: userId ?? '', uploadedAt: Date.now(), - title: (pdfMetadata.title as string) || null, - firstAuthor: (pdfMetadata.firstAuthor as string) || null, - publicationYear: (pdfMetadata.publicationYear as string) || null, - journal: (pdfMetadata.journal as string) || null, - doi: (pdfMetadata.doi as string) || null, + title: (pdfMetadata.title as string) || undefined, + firstAuthor: (pdfMetadata.firstAuthor as string) || undefined, + publicationYear: (pdfMetadata.publicationYear as string) || undefined, + journal: (pdfMetadata.journal as string) || undefined, + doi: (pdfMetadata.doi as string) || undefined, }, - effectiveTag, + effectiveTag as PdfTag, ); - return pdfId; + return pdfId!; } catch (err) { console.error('Error uploading PDF:', err); if (uploadResult?.fileName) { @@ -206,7 +207,7 @@ export const pdfActions = { if (r2Deleted) { try { - ops.removePdfFromStudy(studyId, (pdf as any).id); + ops.pdf.removePdfFromStudy(studyId, (pdf as any).id); } catch (yjsErr) { console.error('Failed to remove PDF from Y.js:', yjsErr); throw new Error('PDF deleted from R2 but failed to remove from study'); @@ -225,13 +226,13 @@ export const pdfActions = { updateTag(studyId: string, pdfId: string, newTag: string): void { const ops = connectionPool.getActiveOps(); if (!ops) throw new Error('No active project connection'); - ops.updatePdfTag(studyId, pdfId, newTag); + ops.pdf.updatePdfTag(studyId, pdfId, newTag as PdfTag); }, updateMetadata(studyId: string, pdfId: string, metadata: Record): void { const ops = connectionPool.getActiveOps(); if (!ops) throw new Error('No active project connection'); - ops.updatePdfMetadata(studyId, pdfId, metadata); + ops.pdf.updatePdfMetadata(studyId, pdfId, metadata); }, async handleGoogleDriveImport(studyId: string, file: any, tag = 'secondary'): Promise { @@ -267,22 +268,21 @@ export const pdfActions = { const pdfMetadata = await extractPdfMetadata(arrayBuffer); - ops.addPdfToStudy( + ops.pdf.addPdfToStudy( studyId, { key: file.key, fileName: file.fileName, size: file.size, - uploadedBy: userId, + uploadedBy: userId ?? '', uploadedAt: Date.now(), - source: 'google-drive', - title: (pdfMetadata.title as string) || null, - firstAuthor: (pdfMetadata.firstAuthor as string) || null, - publicationYear: (pdfMetadata.publicationYear as string) || null, - journal: (pdfMetadata.journal as string) || null, - doi: (pdfMetadata.doi as string) || null, - }, - effectiveTag, + title: (pdfMetadata.title as string) || undefined, + firstAuthor: (pdfMetadata.firstAuthor as string) || undefined, + publicationYear: (pdfMetadata.publicationYear as string) || undefined, + journal: (pdfMetadata.journal as string) || undefined, + doi: (pdfMetadata.doi as string) || undefined, + } as PdfInfo, + effectiveTag as PdfTag, ); } catch (err) { console.error('Failed to add Google Drive PDF metadata:', err); @@ -299,6 +299,6 @@ export const pdfActions = { addToStudy(studyId: string, pdfMeta: Record, tag?: string): void { const ops = connectionPool.getActiveOps(); if (!ops) throw new Error('No active project connection'); - ops.addPdfToStudy(studyId, pdfMeta, tag); + ops.pdf.addPdfToStudy(studyId, pdfMeta as unknown as PdfInfo, tag as PdfTag); }, }; diff --git a/packages/landing/src/project/actions/project.ts b/packages/landing/src/project/actions/project.ts index 2bcf44acf..9b1f3a541 100644 --- a/packages/landing/src/project/actions/project.ts +++ b/packages/landing/src/project/actions/project.ts @@ -13,7 +13,7 @@ export const projectActions = { const ops = connectionPool.getActiveOps(); if (!ops) throw new Error('No active project connection'); try { - await ops.renameProject(newName); + await ops.study.renameProject(newName); } catch (err) { console.error('Error renaming project:', err); showToast.error('Rename Failed', (err as Error).message || 'Failed to rename project'); @@ -24,7 +24,7 @@ export const projectActions = { const ops = connectionPool.getActiveOps(); if (!ops) throw new Error('No active project connection'); try { - await ops.updateDescription(newDescription); + await ops.study.updateDescription(newDescription); } catch (err) { console.error('Error updating description:', err); showToast.error('Update Failed', (err as Error).message || 'Failed to update description'); diff --git a/packages/landing/src/project/actions/reconciliation.ts b/packages/landing/src/project/actions/reconciliation.ts index ed4609e1a..8b3821cf0 100644 --- a/packages/landing/src/project/actions/reconciliation.ts +++ b/packages/landing/src/project/actions/reconciliation.ts @@ -3,23 +3,24 @@ */ import { connectionPool } from '../ConnectionPool'; +import type { ReconciliationProgressData } from '@/primitives/useProject/reconciliation.js'; export const reconciliationActions = { - saveProgress(studyId: string, checklist1Id: string, checklist2Id: string, data: Record) { + saveProgress(studyId: string, outcomeId: string | null, type: string, data: ReconciliationProgressData) { const ops = connectionPool.getActiveOps(); if (!ops) throw new Error('No active project connection'); - return ops.saveReconciliationProgress(studyId, checklist1Id, checklist2Id, data); + return ops.reconciliation.saveReconciliationProgress(studyId, outcomeId, type, data); }, - getProgress(studyId: string, checklist1Id: string, checklist2Id: string) { + getProgress(studyId: string, outcomeId: string | null, type: string) { const ops = connectionPool.getActiveOps(); if (!ops) throw new Error('No active project connection'); - return ops.getReconciliationProgress(studyId, checklist1Id, checklist2Id); + return ops.reconciliation.getReconciliationProgress(studyId, outcomeId, type); }, applyToChecklists(studyId: string, checklist1Id: string, checklist2Id: string, data: Record) { const ops = connectionPool.getActiveOps(); if (!ops) throw new Error('No active project connection'); - return ops.applyReconciliationToChecklists(studyId, checklist1Id, checklist2Id, data); + return (ops.reconciliation as any).applyReconciliationToChecklists(studyId, checklist1Id, checklist2Id, data); }, }; diff --git a/packages/landing/src/project/actions/studies.ts b/packages/landing/src/project/actions/studies.ts index a93a516e6..5b2d2a795 100644 --- a/packages/landing/src/project/actions/studies.ts +++ b/packages/landing/src/project/actions/studies.ts @@ -11,7 +11,9 @@ import { extractPdfDoi, extractPdfTitle } from '@/lib/pdfUtils.js'; import { fetchFromDOI } from '@/lib/referenceLookup.js'; import { useProjectStore } from '@/stores/projectStore'; import { useAuthStore, selectUser } from '@/stores/authStore'; -import { connectionPool, type ConnectionOps } from '../ConnectionPool'; +import { connectionPool, type TypedProjectOps } from '../ConnectionPool'; +import type { PdfInfo, PdfTag } from '@/primitives/useProject/pdfs'; +import type { StudyMetadata } from '@/primitives/useProject/studies'; // --------------------------------------------------------------------------- // Pure helpers (no pool dependency) @@ -73,7 +75,7 @@ function filterDefinedUpdates(updates: Record): Record, orgId: string, @@ -81,7 +83,7 @@ async function addPdfMetadataToStudy( tag = 'primary', ): Promise { try { - ops.addPdfToStudy(studyId, pdfInfo, tag); + ops.pdf.addPdfToStudy(studyId, pdfInfo as unknown as PdfInfo, tag as PdfTag); return true; } catch (err) { console.error('Failed to add PDF metadata:', err); @@ -96,7 +98,7 @@ async function addPdfMetadataToStudy( } async function handleGoogleDrivePdf( - ops: ConnectionOps, + ops: TypedProjectOps, study: Record, studyId: string, orgId: string, @@ -149,7 +151,7 @@ async function handleGoogleDrivePdf( const filtered = filterDefinedUpdates(metadataUpdates); if (Object.keys(filtered).length > 0) { - ops.updateStudy(studyId, filtered); + ops.study.updateStudy(studyId, filtered); } } catch (extractErr) { console.warn('Failed to extract metadata for Google Drive PDF:', extractErr); @@ -180,7 +182,7 @@ async function fetchPdfFromUrl( } async function uploadAndAttachPdf( - ops: ConnectionOps, + ops: TypedProjectOps, pdfData: ArrayBuffer, pdfFileName: string, studyId: string, @@ -258,14 +260,14 @@ export const studyActions = { create(name: string, description = '', metadata: Record = {}): string | null { const ops = connectionPool.getActiveOps(); if (!ops) throw new Error('No active project connection'); - return ops.createStudy(name, description, metadata); + return ops.study.createStudy(name, description, metadata); }, update(studyId: string, updates: Record): void { const ops = connectionPool.getActiveOps(); if (!ops) throw new Error('No active project connection'); try { - ops.updateStudy(studyId, updates); + ops.study.updateStudy(studyId, updates); } catch (err) { console.error('Error updating study:', err); showToast.error('Update Failed', 'Failed to update study'); @@ -306,7 +308,7 @@ export const studyActions = { studyId, }); - ops.deleteStudy(studyId); + ops.study.deleteStudy(studyId); } catch (err) { console.error('Error deleting study:', err); showToast.error('Delete Failed', 'Failed to delete study'); @@ -332,21 +334,21 @@ export const studyActions = { try { for (const study of studiesToAdd) { try { - const metadata = { - originalTitle: study.title || study.name || null, - firstAuthor: study.firstAuthor, - publicationYear: study.publicationYear, - authors: study.authors, - journal: study.journal, - doi: study.doi, - abstract: study.abstract, - importSource: study.importSource, - pdfUrl: study.pdfUrl, - pdfSource: study.pdfSource, + const metadata: StudyMetadata = { + originalTitle: (study.title as string) || (study.name as string) || undefined, + firstAuthor: (study.firstAuthor as string) || undefined, + publicationYear: (study.publicationYear as string) || undefined, + authors: (study.authors as string) || undefined, + journal: (study.journal as string) || undefined, + doi: (study.doi as string) || undefined, + abstract: (study.abstract as string) || undefined, + importSource: (study.importSource as string) || undefined, + pdfUrl: (study.pdfUrl as string) || undefined, + pdfSource: (study.pdfSource as string) || undefined, }; const studyName = getStudyNameFromFilename(study.pdfFileName as string | null); - const studyId = ops.createStudy(studyName, (study.abstract as string) || '', metadata); + const studyId = ops.study.createStudy(studyName, (study.abstract as string) || '', metadata); if (!studyId) continue; let pdfAttached = false; @@ -416,14 +418,14 @@ export const studyActions = { ? getStudyNameFromFilename(ref.pdfFileName as string) : (ref.title as string) || 'Untitled Study'; - ops.createStudy(studyName, (ref.abstract as string) || '', { - originalTitle: ref.title || null, - firstAuthor: ref.firstAuthor, - publicationYear: ref.publicationYear, - authors: ref.authors, - journal: ref.journal, - doi: ref.doi, - abstract: ref.abstract, + ops.study.createStudy(studyName, (ref.abstract as string) || '', { + originalTitle: (ref.title as string) || undefined, + firstAuthor: (ref.firstAuthor as string) || undefined, + publicationYear: (ref.publicationYear as string) || undefined, + authors: (ref.authors as string) || undefined, + journal: (ref.journal as string) || undefined, + doi: (ref.doi as string) || undefined, + abstract: (ref.abstract as string) || undefined, importSource: 'reference-file', }); successCount++; From cb1b90415009452ca4dd6d160718755ee7181988 Mon Sep 17 00:00:00 2001 From: Jacob Maynard Date: Sat, 28 Mar 2026 21:38:34 -0500 Subject: [PATCH 3/7] phase 3 --- packages/landing/src/project/actions.ts | 138 +++++++++++++++++- .../landing/src/project/actions/checklists.ts | 83 ----------- .../landing/src/project/actions/outcomes.ts | 37 ----- .../src/project/actions/reconciliation.ts | 26 ---- .../landing/src/project/connectionReducer.ts | 26 ---- 5 files changed, 131 insertions(+), 179 deletions(-) delete mode 100644 packages/landing/src/project/actions/checklists.ts delete mode 100644 packages/landing/src/project/actions/outcomes.ts delete mode 100644 packages/landing/src/project/actions/reconciliation.ts diff --git a/packages/landing/src/project/actions.ts b/packages/landing/src/project/actions.ts index 16757ee37..9f41febe7 100644 --- a/packages/landing/src/project/actions.ts +++ b/packages/landing/src/project/actions.ts @@ -1,26 +1,150 @@ /** * Typed project actions singleton. * Components use `project.study.create(...)` for write operations. - * All action modules import connectionPool directly. */ +import { showToast } from '@/components/ui/toast'; +import { useAuthStore, selectUser } from '@/stores/authStore'; import { connectionPool } from './ConnectionPool'; import { studyActions } from './actions/studies'; -import { checklistActions } from './actions/checklists'; import { pdfActions } from './actions/pdfs'; import { projectActions } from './actions/project'; import { memberActions } from './actions/members'; -import { reconciliationActions } from './actions/reconciliation'; -import { outcomeActions } from './actions/outcomes'; +import type { ReconciliationProgressData } from '@/primitives/useProject/reconciliation.js'; export const project = { study: studyActions, - checklist: checklistActions, pdf: pdfActions, project: projectActions, member: memberActions, - reconciliation: reconciliationActions, - outcome: outcomeActions, + + checklist: { + create(studyId: string, type: string, assigneeId: string | null, outcomeId?: string): boolean { + const ops = connectionPool.getActiveOps(); + if (!ops) throw new Error('No active project connection'); + try { + const checklistId = ops.checklist.createChecklist(studyId, type, assigneeId, outcomeId ?? null); + if (!checklistId) { + const requiresOutcome = type === 'ROB2' || type === 'ROBINS_I'; + if (requiresOutcome && !outcomeId) { + showToast.error('Addition Failed', `${type} requires an outcome to be selected`); + } else if (requiresOutcome) { + showToast.error( + 'Addition Failed', + 'You already have a checklist for this outcome. Select a different outcome.', + ); + } else { + showToast.error('Addition Failed', 'Failed to add checklist'); + } + return false; + } + return true; + } catch (err) { + console.error('Error adding checklist:', err); + showToast.error('Addition Failed', 'Failed to add checklist'); + return false; + } + }, + + update(studyId: string, checklistId: string, updates: Record): void { + const ops = connectionPool.getActiveOps(); + if (!ops) throw new Error('No active project connection'); + try { + ops.checklist.updateChecklist(studyId, checklistId, updates); + } catch (err) { + console.error('Error updating checklist:', err); + showToast.error('Update Failed', 'Failed to update checklist'); + } + }, + + delete(studyId: string, checklistId: string): void { + const ops = connectionPool.getActiveOps(); + if (!ops) throw new Error('No active project connection'); + try { + ops.checklist.deleteChecklist(studyId, checklistId); + } catch (err) { + console.error('Error deleting checklist:', err); + showToast.error('Delete Failed', 'Failed to delete checklist'); + } + }, + + getAnswersMap(studyId: string, checklistId: string): unknown { + const ops = connectionPool.getActiveOps(); + if (!ops) throw new Error('No active project connection'); + return ops.checklist.getChecklistAnswersMap(studyId, checklistId); + }, + + getData(studyId: string, checklistId: string): Record | null { + const ops = connectionPool.getActiveOps(); + if (!ops) throw new Error('No active project connection'); + return ops.checklist.getChecklistData(studyId, checklistId) ?? null; + }, + + updateAnswer(studyId: string, checklistId: string, questionId: string, answer: unknown, note?: string): void { + const ops = connectionPool.getActiveOps(); + if (!ops) throw new Error('No active project connection'); + const data: Record = { answer }; + if (note !== undefined) data.note = note; + ops.checklist.updateChecklistAnswer(studyId, checklistId, questionId, data); + }, + + getQuestionNote(studyId: string, checklistId: string, questionId: string): unknown { + const ops = connectionPool.getActiveOps(); + if (!ops) throw new Error('No active project connection'); + return ops.checklist.getQuestionNote(studyId, checklistId, questionId); + }, + }, + + outcome: { + create(name: string): string | null { + const conn = connectionPool.getActiveOps(); + if (!conn) throw new Error('No active project connection'); + const user = selectUser(useAuthStore.getState()); + if (!user?.id) { + console.error('[outcome.create] No user logged in'); + return null; + } + return conn.outcome.createOutcome(name, user.id); + }, + + update(outcomeId: string, name: string): boolean { + const conn = connectionPool.getActiveOps(); + if (!conn) throw new Error('No active project connection'); + return conn.outcome.updateOutcome(outcomeId, name); + }, + + delete(outcomeId: string): { success: boolean; error?: string } { + const conn = connectionPool.getActiveOps(); + if (!conn) throw new Error('No active project connection'); + return conn.outcome.deleteOutcome(outcomeId); + }, + + isInUse(outcomeId: string): boolean { + const conn = connectionPool.getActiveOps(); + if (!conn) throw new Error('No active project connection'); + return conn.outcome.isOutcomeInUse(outcomeId); + }, + }, + + reconciliation: { + saveProgress(studyId: string, outcomeId: string | null, type: string, data: ReconciliationProgressData) { + const ops = connectionPool.getActiveOps(); + if (!ops) throw new Error('No active project connection'); + return ops.reconciliation.saveReconciliationProgress(studyId, outcomeId, type, data); + }, + + getProgress(studyId: string, outcomeId: string | null, type: string) { + const ops = connectionPool.getActiveOps(); + if (!ops) throw new Error('No active project connection'); + return ops.reconciliation.getReconciliationProgress(studyId, outcomeId, type); + }, + + applyToChecklists(studyId: string, checklist1Id: string, checklist2Id: string, data: Record) { + const ops = connectionPool.getActiveOps(); + if (!ops) throw new Error('No active project connection'); + return (ops.reconciliation as any).applyReconciliationToChecklists(studyId, checklist1Id, checklist2Id, data); + }, + }, getActiveProjectId: () => connectionPool.getActiveProjectId(), getActiveOrgId: () => connectionPool.getActiveOrgId(), diff --git a/packages/landing/src/project/actions/checklists.ts b/packages/landing/src/project/actions/checklists.ts deleted file mode 100644 index 1a5b0d077..000000000 --- a/packages/landing/src/project/actions/checklists.ts +++ /dev/null @@ -1,83 +0,0 @@ -/** - * Checklist actions -- create, update, delete, get data - */ - -import { showToast } from '@/components/ui/toast'; -import { connectionPool } from '../ConnectionPool'; - -export const checklistActions = { - create(studyId: string, type: string, assigneeId: string | null, outcomeId?: string): boolean { - const ops = connectionPool.getActiveOps(); - if (!ops) throw new Error('No active project connection'); - try { - const checklistId = ops.checklist.createChecklist(studyId, type, assigneeId, outcomeId ?? null); - if (!checklistId) { - const requiresOutcome = type === 'ROB2' || type === 'ROBINS_I'; - if (requiresOutcome && !outcomeId) { - showToast.error('Addition Failed', `${type} requires an outcome to be selected`); - } else if (requiresOutcome) { - showToast.error( - 'Addition Failed', - 'You already have a checklist for this outcome. Select a different outcome.', - ); - } else { - showToast.error('Addition Failed', 'Failed to add checklist'); - } - return false; - } - return true; - } catch (err) { - console.error('Error adding checklist:', err); - showToast.error('Addition Failed', 'Failed to add checklist'); - return false; - } - }, - - update(studyId: string, checklistId: string, updates: Record): void { - const ops = connectionPool.getActiveOps(); - if (!ops) throw new Error('No active project connection'); - try { - ops.checklist.updateChecklist(studyId, checklistId, updates); - } catch (err) { - console.error('Error updating checklist:', err); - showToast.error('Update Failed', 'Failed to update checklist'); - } - }, - - delete(studyId: string, checklistId: string): void { - const ops = connectionPool.getActiveOps(); - if (!ops) throw new Error('No active project connection'); - try { - ops.checklist.deleteChecklist(studyId, checklistId); - } catch (err) { - console.error('Error deleting checklist:', err); - showToast.error('Delete Failed', 'Failed to delete checklist'); - } - }, - - getAnswersMap(studyId: string, checklistId: string): unknown { - const ops = connectionPool.getActiveOps(); - if (!ops) throw new Error('No active project connection'); - return ops.checklist.getChecklistAnswersMap(studyId, checklistId); - }, - - getData(studyId: string, checklistId: string): Record | null { - const ops = connectionPool.getActiveOps(); - if (!ops) throw new Error('No active project connection'); - return ops.checklist.getChecklistData(studyId, checklistId) ?? null; - }, - - updateAnswer(studyId: string, checklistId: string, questionId: string, answer: unknown, note?: string): void { - const ops = connectionPool.getActiveOps(); - if (!ops) throw new Error('No active project connection'); - const data: Record = { answer }; - if (note !== undefined) data.note = note; - ops.checklist.updateChecklistAnswer(studyId, checklistId, questionId, data); - }, - - getQuestionNote(studyId: string, checklistId: string, questionId: string): unknown { - const ops = connectionPool.getActiveOps(); - if (!ops) throw new Error('No active project connection'); - return ops.checklist.getQuestionNote(studyId, checklistId, questionId); - }, -}; diff --git a/packages/landing/src/project/actions/outcomes.ts b/packages/landing/src/project/actions/outcomes.ts deleted file mode 100644 index 4f0961523..000000000 --- a/packages/landing/src/project/actions/outcomes.ts +++ /dev/null @@ -1,37 +0,0 @@ -/** - * Outcome actions -- create, update, delete project-level outcomes - */ - -import { useAuthStore, selectUser } from '@/stores/authStore'; -import { connectionPool } from '../ConnectionPool'; - -export const outcomeActions = { - create(name: string): string | null { - const conn = connectionPool.getActiveOps(); - if (!conn) throw new Error('No active project connection'); - const user = selectUser(useAuthStore.getState()); - if (!user?.id) { - console.error('[outcome.create] No user logged in'); - return null; - } - return conn.outcome.createOutcome(name, user.id); - }, - - update(outcomeId: string, name: string): boolean { - const conn = connectionPool.getActiveOps(); - if (!conn) throw new Error('No active project connection'); - return conn.outcome.updateOutcome(outcomeId, name); - }, - - delete(outcomeId: string): { success: boolean; error?: string } { - const conn = connectionPool.getActiveOps(); - if (!conn) throw new Error('No active project connection'); - return conn.outcome.deleteOutcome(outcomeId); - }, - - isInUse(outcomeId: string): boolean { - const conn = connectionPool.getActiveOps(); - if (!conn) throw new Error('No active project connection'); - return conn.outcome.isOutcomeInUse(outcomeId); - }, -}; diff --git a/packages/landing/src/project/actions/reconciliation.ts b/packages/landing/src/project/actions/reconciliation.ts deleted file mode 100644 index 8b3821cf0..000000000 --- a/packages/landing/src/project/actions/reconciliation.ts +++ /dev/null @@ -1,26 +0,0 @@ -/** - * Reconciliation actions -- save/load reconciliation progress - */ - -import { connectionPool } from '../ConnectionPool'; -import type { ReconciliationProgressData } from '@/primitives/useProject/reconciliation.js'; - -export const reconciliationActions = { - saveProgress(studyId: string, outcomeId: string | null, type: string, data: ReconciliationProgressData) { - const ops = connectionPool.getActiveOps(); - if (!ops) throw new Error('No active project connection'); - return ops.reconciliation.saveReconciliationProgress(studyId, outcomeId, type, data); - }, - - getProgress(studyId: string, outcomeId: string | null, type: string) { - const ops = connectionPool.getActiveOps(); - if (!ops) throw new Error('No active project connection'); - return ops.reconciliation.getReconciliationProgress(studyId, outcomeId, type); - }, - - applyToChecklists(studyId: string, checklist1Id: string, checklist2Id: string, data: Record) { - const ops = connectionPool.getActiveOps(); - if (!ops) throw new Error('No active project connection'); - return (ops.reconciliation as any).applyReconciliationToChecklists(studyId, checklist1Id, checklist2Id, data); - }, -}; diff --git a/packages/landing/src/project/connectionReducer.ts b/packages/landing/src/project/connectionReducer.ts index 85ef2522c..457bf0c07 100644 --- a/packages/landing/src/project/connectionReducer.ts +++ b/packages/landing/src/project/connectionReducer.ts @@ -95,29 +95,3 @@ export function connectionReducer( export { INITIAL_STATE }; -/** - * Map the new phase enum to the legacy 4-boolean shape for backward compatibility. - * Remove once all consumers are migrated to check `phase` directly. - */ -export function phaseToLegacy(state: ConnectionMachineState): { - connected: boolean; - connecting: boolean; - synced: boolean; - error: string | null; -} { - switch (state.phase) { - case 'idle': - return { connected: false, connecting: false, synced: false, error: null }; - case 'loading': - case 'connecting': - return { connected: false, connecting: true, synced: false, error: null }; - case 'connected': - return { connected: true, connecting: false, synced: false, error: null }; - case 'synced': - return { connected: true, connecting: false, synced: true, error: null }; - case 'offline': - return { connected: false, connecting: false, synced: false, error: null }; - case 'error': - return { connected: false, connecting: false, synced: false, error: state.error }; - } -} From 492566549d7c48b6de9c142e03a1524785a22fdc Mon Sep 17 00:00:00 2001 From: GitHub Actions Date: Sun, 29 Mar 2026 02:48:46 +0000 Subject: [PATCH 4/7] Apply Prettier formatting --- .../audits/landing-simplify-2026-03-21.md | 7 + .../docs/audits/rob2-visualizations-plan.md | 43 ++-- .../audits/yjs-sync-pipeline-redesign-a.md | 224 +++++++++--------- .../audits/yjs-sync-pipeline-redesign-b.md | 119 +++++----- packages/landing/e2e/auth-flow.spec.ts | 16 +- packages/landing/e2e/rob2-workflow.spec.ts | 8 +- .../src/components/auth/RoleSelector.tsx | 2 - .../src/components/auth/StrengthIndicator.tsx | 2 - .../components/charts/AMSTARDistribution.tsx | 3 +- .../src/components/charts/AMSTARRobvis.tsx | 4 +- .../checklist/ChecklistYjsWrapper.tsx | 16 +- .../ROBINSIChecklist/checklist-compare.ts | 6 +- .../checklist/SplitPanelControls.tsx | 2 - .../checklist/common/LocalTextAdapter.ts | 33 ++- .../dashboard/LocalAppraisalCard.tsx | 2 - .../src/components/dashboard/ProjectCard.tsx | 2 - .../components/dashboard/ProjectsSection.tsx | 2 - .../dashboard/useInitialAnimation.ts | 2 - .../landing/src/components/dev/DevPanel.tsx | 24 +- .../src/components/dev/DevStudyGenerator.tsx | 3 +- .../src/components/layout/SettingsSidebar.tsx | 2 - .../landing/src/components/layout/Sidebar.tsx | 2 - .../layout/sidebar/LocalChecklistItem.tsx | 2 - .../layout/sidebar/ProjectTreeItem.tsx | 3 +- .../src/components/pdf/PdfListItem.tsx | 1 - .../react/src/components/search-sidebar.tsx | 2 +- .../react/src/hooks/useAnnotationSync.ts | 20 +- .../src/components/project/ProjectContext.tsx | 2 - .../src/components/project/ProjectHeader.tsx | 2 - .../src/components/project/SlidingPanel.tsx | 1 - .../project/add-studies/AddStudiesForm.tsx | 2 - .../all-studies-tab/AssignReviewersModal.tsx | 2 - .../all-studies-tab/EditPdfMetadataModal.tsx | 1 - .../all-studies-tab/study-card/StudyCard.tsx | 1 - .../study-card/StudyCardHeader.tsx | 1 - .../study-card/StudyPdfSection.tsx | 9 +- .../completed-tab/CompletedOutcomeRow.tsx | 2 - .../completed-tab/CompletedStudyRow.tsx | 2 - .../completed-tab/PreviousReviewersView.tsx | 14 +- .../GoogleDrivePickerLauncher.tsx | 2 - .../project/overview-tab/ChartSection.tsx | 6 +- .../overview-tab/ReviewerAssignment.tsx | 2 - .../reconcile-tab/ReconcileStatusTag.tsx | 2 - .../reconcile-tab/ReconcileStudyRow.tsx | 2 - .../reconcile-tab/ReconciliationWrapper.tsx | 23 +- .../amstar2-reconcile/navbar-utils.ts | 4 +- .../rob2-reconcile/navbar-utils.ts | 9 +- .../rob2-reconcile/pages/PreliminaryPage.tsx | 3 +- .../robins-i-reconcile/navbar-utils.ts | 4 +- .../project/todo-tab/ChecklistForm.tsx | 2 - .../project/todo-tab/TodoStudyRow.tsx | 2 - .../settings/MergeAccountsDialog.tsx | 2 - .../components/settings/SessionManagement.tsx | 2 - .../landing/src/components/ui/editable.tsx | 2 - .../landing/src/hooks/useAddStudies/index.ts | 10 +- .../landing/src/hooks/useAddStudies/lookup.ts | 21 +- .../src/hooks/useAddStudies/references.ts | 12 +- .../src/hooks/useAddStudies/serialization.ts | 5 +- .../landing/src/hooks/useNotifications.ts | 2 - packages/landing/src/lib/checklist-domain.ts | 14 +- packages/landing/src/lib/embedPdfEngine.ts | 8 +- packages/landing/src/lib/errorLogger.ts | 8 +- .../landing/src/lib/formStatePersistence.ts | 6 +- packages/landing/src/lib/googlePicker.ts | 20 +- .../src/lib/inter-rater-reliability.ts | 5 +- packages/landing/src/lib/pdfUtils.ts | 8 +- packages/landing/src/lib/referenceLookup.ts | 4 +- packages/landing/src/lib/yjsUtils.ts | 12 +- .../src/primitives/useProject/annotations.ts | 9 +- .../src/primitives/useProject/connection.ts | 95 ++++---- .../src/primitives/useProject/outcomes.ts | 5 +- .../landing/src/primitives/useProject/pdfs.ts | 6 +- .../primitives/useProject/reconciliation.ts | 21 +- .../landing/src/primitives/useProject/sync.ts | 16 +- .../landing/src/project/ConnectionPool.ts | 16 +- packages/landing/src/project/ProjectGate.tsx | 6 +- packages/landing/src/project/actions.ts | 36 ++- packages/landing/src/project/actions/pdfs.ts | 10 +- .../landing/src/project/actions/studies.ts | 26 +- .../landing/src/project/connectionReducer.ts | 1 - packages/landing/src/router.tsx | 1 - .../_app/_protected/admin/orgs.$orgId.tsx | 166 ++++++------- packages/landing/src/stores/adminStore.ts | 2 - packages/landing/src/stores/authStore.ts | 2 - .../src/stores/localChecklistsStore.ts | 2 - .../landing/src/stores/pdfPreviewStore.ts | 2 - packages/landing/src/stores/projectStore.ts | 2 - .../src/__tests__/cloudflare-test.d.ts | 1 - packages/workers/src/auth/config.ts | 27 ++- .../src/durable-objects/dev-handlers.ts | 44 +++- packages/workers/src/lib/mock-templates.ts | 13 +- packages/workers/src/routes/test-seed.ts | 6 +- 92 files changed, 701 insertions(+), 607 deletions(-) diff --git a/packages/docs/audits/landing-simplify-2026-03-21.md b/packages/docs/audits/landing-simplify-2026-03-21.md index bc3722b1b..c019310fc 100644 --- a/packages/docs/audits/landing-simplify-2026-03-21.md +++ b/packages/docs/audits/landing-simplify-2026-03-21.md @@ -19,42 +19,49 @@ Full review of `/packages/landing/src` (477 source files) for code reuse, qualit ## Bugs Found and Fixed (discovered during audit work) ### B1. ROB2 optional question marking too aggressive + - **Files**: `components/checklist/ROB2Checklist/DomainSection.tsx` - **Problem**: Questions 4.2-4.5 showed "(Optional)" when no answers were given. `getRequiredQuestions` returns only the entry-point question with no answers, so everything else looked optional. - **Fix**: Added `hasAnyAnswer` guard -- questions only marked optional once the user has started answering in that domain. - [x] Fixed ### B2. Immer stack overflow on reconciliation "Use This" + - **Files**: `stores/projectStore.ts`, `primitives/useProject/sync.ts` - **Problem**: `persistStats` called `JSON.stringify` on an Immer draft proxy inside `produce`, causing stack overflow. Also ROB2 Y.Text objects leaked into Immer via `toJSON()`. - **Fix**: Moved `persistStats` outside Immer `produce`. Added explicit ROB2 Y.Text serialization in `sync.ts`. - [x] Fixed ### B3. ROB2 reconciliation navbar pills not expanding + - **Files**: `components/project/reconcile-tab/rob2-reconcile/NavbarDomainPill.tsx`, `engine/useReconciliationEngine.ts`, `engine/types.ts`, all adapter navbar-utils - **Problem**: Two issues: (1) Radix Collapsible only supports vertical expand, not horizontal. (2) Engine stored `item.section` (display name like "Domain 1: Bias...") but navbar compared against section keys ("domain1") -- they never matched. - **Fix**: Replaced Radix Collapsible with CSS `max-width` transition. Added `sectionKey` field to `ReconciliationNavItem`. All adapters and navbar-utils now use `sectionKey` consistently. - [x] Fixed ### B4. PreliminaryPage Y.Text value bleeding between fields + - **Files**: `components/project/reconcile-tab/rob2-reconcile/pages/PreliminaryPage.tsx`, `adapter.tsx` - **Problem**: When navigating between preliminary text field pages, the `onFinalChange` closure captured the new field's key but the effect fired with the old field's Y.Text value, writing it to the wrong field. - **Fix**: Added `key={currentItem.key}` on PreliminaryPage in the adapter (forces remount per field). Used `useEffectEvent` for the sync callback to avoid stale closures. - [x] Fixed ### B5. Aim selection clearing text fields in ROB2 checklist + - **Files**: `components/checklist/ROB2Checklist/PreliminarySection.tsx` - **Problem**: All preliminary handlers spread `...preliminaryState` when updating, overwriting Y.Text fields with stale/empty string values. Clicking the aim radio after typing text wiped the text. - **Fix**: Changed handlers to only send the changed field (e.g. `{ aim: value }` instead of `{ ...preliminaryState, aim: value }`). The ROB2 handler's `updateAnswer` already does field-level merging. - [x] Fixed ### B6. Presence not working in ROB2 reconciliation + - **Files**: `project/ConnectionPool.ts` - **Problem**: `getAwareness` was a method on the `ConnectionPool` class but was not included in `buildOpsMap`. `ReconciliationWrapper` destructured it from the ops map and got `undefined`. - **Fix**: Added `getAwareness` to the flat ops map in `buildOpsMap`. - [x] Fixed ### B7. ROB2 e2e test only smoke-tested reconciliation + - **Files**: `e2e/rob2-workflow.spec.ts` - **Problem**: Test verified reconciliation page loaded but never clicked "Use This", navigated through items, or saved. Would not have caught B2, B3, B4, or B5. - **Fix**: Extended test to walk through all 34 reconciliation items (clicking "Use This", selecting directions, checking sources), verify summary, save, and confirm finalization. diff --git a/packages/docs/audits/rob2-visualizations-plan.md b/packages/docs/audits/rob2-visualizations-plan.md index 00788e31c..d2872975f 100644 --- a/packages/docs/audits/rob2-visualizations-plan.md +++ b/packages/docs/audits/rob2-visualizations-plan.md @@ -5,18 +5,19 @@ The project has AMSTAR2 visualizations (traffic light heatmap + distribution stacked bar chart) but no ROB2 equivalents. ROB2 assessments are fully functional (checklist, scoring, reconciliation) but there's no way to visualize finalized ROB2 results at the project overview level. Standard robvis-style ROB2 visualizations show: + - **Traffic light plot**: rows = studies, columns = 5 bias domains + Overall - **Summary plot**: horizontal stacked bars showing % Low / Some concerns / High per domain ## Data Differences: ROB2 vs AMSTAR2 -| Aspect | AMSTAR2 | ROB2 | -|--------|---------|------| -| Columns | 16 flat questions (Q1-Q16) | 5 domains + Overall (6 columns) | -| Values | yes, partial yes, no, no ma | Low, Some concerns, High | -| Legend items | 4 | 3 | -| Branching | None | Domain 2a vs 2b based on aim | -| consolidatedAnswers | Set in sync.ts via `getAMSTAR2Answers()` | **Not currently set** | +| Aspect | AMSTAR2 | ROB2 | +| ------------------- | ---------------------------------------- | ------------------------------- | +| Columns | 16 flat questions (Q1-Q16) | 5 domains + Overall (6 columns) | +| Values | yes, partial yes, no, no ma | Low, Some concerns, High | +| Legend items | 4 | 3 | +| Branching | None | Domain 2a vs 2b based on aim | +| consolidatedAnswers | Set in sync.ts via `getAMSTAR2Answers()` | **Not currently set** | ## Implementation Plan @@ -30,7 +31,7 @@ Add a new function that extracts domain-level judgments into a chart-friendly fo export interface ROB2ConsolidatedAnswers { aim: 'ASSIGNMENT' | 'ADHERING' | null; domain2Variant: '2a' | '2b'; - judgments: (string | null)[]; // [D1, D2, D3, D4, D5, Overall] -- 6 entries + judgments: (string | null)[]; // [D1, D2, D3, D4, D5, Overall] -- 6 entries } ``` @@ -59,6 +60,7 @@ Move the `exportChart` function from `ChartSection.tsx` into a shared utility. U **File (new)**: `packages/landing/src/components/charts/ROB2Robvis.tsx` D3-based SVG component mirroring `AMSTARRobvis.tsx` with these differences: + - **6 columns**: D1, D2, D3, D4, D5, Overall (instead of 16) - **Color map**: `{ low: '#10b981', 'some concerns': '#facc15', high: '#ef4444' }` - **Greyscale map**: `{ low: '#1b1b1b', 'some concerns': '#484848', high: '#727272' }` @@ -69,10 +71,11 @@ D3-based SVG component mirroring `AMSTARRobvis.tsx` with these differences: - Same patterns: `useImperativeHandle`, `ResizeObserver`, `useLayoutEffect` for label width, D3 imperative draw Data interface: + ```typescript interface ROB2RobvisDataItem { label: string; - judgments: (string | null)[]; // 6 entries + judgments: (string | null)[]; // 6 entries } ``` @@ -81,6 +84,7 @@ interface ROB2RobvisDataItem { **File (new)**: `packages/landing/src/components/charts/ROB2Distribution.tsx` D3-based SVG component mirroring `AMSTARDistribution.tsx` with these differences: + - **6 bars** (D1-D5 + Overall) instead of 16 - **3 stacked categories** (Low, Some concerns, High) instead of 4 - Same color/greyscale maps as ROB2Robvis @@ -93,6 +97,7 @@ D3-based SVG component mirroring `AMSTARDistribution.tsx` with these differences **File (new)**: `packages/landing/src/components/project/overview-tab/ROB2ChartSection.tsx` Parallel to existing `ChartSection.tsx`. Responsibilities: + - Filter studies for finalized ROB2 checklists (`type === 'ROB2'`, `status === CHECKLIST_STATUS.FINALIZED`) - Extract `consolidatedAnswers.judgments` from each - Manage state: custom labels, greyscale, titles, transparent export, settings modal @@ -109,16 +114,16 @@ Add `ROB2ChartSection` alongside `ChartSection` in the Figures collapsible secti ## Files Modified (summary) -| File | Action | -|------|--------| -| `packages/shared/src/checklists/rob2/answers.ts` | Add `getConsolidatedAnswers` | -| `packages/landing/src/primitives/useProject/sync.ts` | Add ROB2 consolidation block | -| `packages/landing/src/components/charts/export-chart.ts` | **New** -- extracted utility | -| `packages/landing/src/components/charts/ROB2Robvis.tsx` | **New** -- traffic light chart | -| `packages/landing/src/components/charts/ROB2Distribution.tsx` | **New** -- distribution chart | -| `packages/landing/src/components/project/overview-tab/ROB2ChartSection.tsx` | **New** -- orchestrator | -| `packages/landing/src/components/project/overview-tab/OverviewTab.tsx` | Add ROB2ChartSection | -| `packages/landing/src/components/project/overview-tab/ChartSection.tsx` | Import exportChart from shared util | +| File | Action | +| --------------------------------------------------------------------------- | ----------------------------------- | +| `packages/shared/src/checklists/rob2/answers.ts` | Add `getConsolidatedAnswers` | +| `packages/landing/src/primitives/useProject/sync.ts` | Add ROB2 consolidation block | +| `packages/landing/src/components/charts/export-chart.ts` | **New** -- extracted utility | +| `packages/landing/src/components/charts/ROB2Robvis.tsx` | **New** -- traffic light chart | +| `packages/landing/src/components/charts/ROB2Distribution.tsx` | **New** -- distribution chart | +| `packages/landing/src/components/project/overview-tab/ROB2ChartSection.tsx` | **New** -- orchestrator | +| `packages/landing/src/components/project/overview-tab/OverviewTab.tsx` | Add ROB2ChartSection | +| `packages/landing/src/components/project/overview-tab/ChartSection.tsx` | Import exportChart from shared util | ## Verification diff --git a/packages/docs/audits/yjs-sync-pipeline-redesign-a.md b/packages/docs/audits/yjs-sync-pipeline-redesign-a.md index d2009b493..cdf2ef8c4 100644 --- a/packages/docs/audits/yjs-sync-pipeline-redesign-a.md +++ b/packages/docs/audits/yjs-sync-pipeline-redesign-a.md @@ -90,14 +90,14 @@ Zustand notifies all subscribers -> React re-renders ### File Inventory -| File | Lines | Responsibility | -|------|-------|---------------| -| `ConnectionPool.ts` | 373 | God object: lifecycle, ops, persistence, WebSocket, cleanup | -| `connection.ts` | 286 | WebSocket management wrapping y-websocket | -| `sync.ts` | 505 | Full-tree Y.Doc-to-plain-JS extraction | -| `connectionReducer.ts` | 124 | Connection state machine | -| `projectStore.ts` | 274 | Zustand store: projects, connections, selectors | -| `ProjectDoc.ts` | 812 | Server-side Durable Object | +| File | Lines | Responsibility | +| ---------------------- | ----- | ----------------------------------------------------------- | +| `ConnectionPool.ts` | 373 | God object: lifecycle, ops, persistence, WebSocket, cleanup | +| `connection.ts` | 286 | WebSocket management wrapping y-websocket | +| `sync.ts` | 505 | Full-tree Y.Doc-to-plain-JS extraction | +| `connectionReducer.ts` | 124 | Connection state machine | +| `projectStore.ts` | 274 | Zustand store: projects, connections, selectors | +| `ProjectDoc.ts` | 812 | Server-side Durable Object | ### Data Consumption Patterns @@ -124,6 +124,7 @@ Four systems were studied for transferable patterns: TanStack DB, Zero (Rocicorp **Model**: Client-side normalized collections with live queries. Two-layer state (synced + optimistic). SQL-like query expressions compiled to an IVM (Incremental View Maintenance) pipeline. **Transferable patterns**: + - `useLiveQuery` ergonomics for declarative reactive subscriptions - `$synced` virtual property on rows to track sync status - Transaction-based mutation batching @@ -136,6 +137,7 @@ Four systems were studied for transferable patterns: TanStack DB, Zero (Rocicorp **Model**: Local-first reactive database backed by PostgreSQL. Queries compiled to IVM operator pipeline on the client. Server maintains Client View Records (CVR) tracking what each client has seen. **Transferable patterns**: + - IVM principle: don't recompute everything when one thing changes - The split between "what queries are subscribed" vs "what data exists" - Generator-based streaming for responsiveness @@ -147,6 +149,7 @@ Four systems were studied for transferable patterns: TanStack DB, Zero (Rocicorp **Model**: Server-driven reactive database. Clients subscribe to query functions. Server re-executes queries when underlying data changes and pushes new results via Transitions. **Transferable patterns**: + - Three-tier API layering: Base client -> Framework-agnostic -> React hooks - Optimistic updates with replay semantics - Version-tracked subscription invalidation @@ -162,6 +165,7 @@ Four systems were studied for transferable patterns: TanStack DB, Zero (Rocicorp **Why this does not directly apply to CoRATES**: CoRATES puts structured data (checklist answers, study metadata, member lists, PDF references, annotations, reconciliation progress) in Y.Doc -- not just editor content. Outline avoids this problem entirely by keeping all non-editor data in MobX. CoRATES cannot adopt this approach without losing real-time collaboration on checklists. **Transferable patterns**: + - Direct binding to Y.Text for collaborative text fields (notes, comments) - Being intentional about what goes in Y.Doc and using the right observation strategy for each data category @@ -176,6 +180,7 @@ Y.js already has the mechanism: `Y.Map.observeDeep()` fires once per transaction ### Should CoRATES restructure data as a relational store? No. Y.js is a document CRDT. Making it act like a relational store would mean fighting its core abstraction: + - Y.js has no concept of tables, foreign keys, or joins - Simulating relations with top-level Y.Maps would require manual referential integrity - Y.Text collaborative editing requires a parent Y.Doc -- normalizing answers into flat "tables" breaks Y.Text's natural home in the document tree @@ -314,18 +319,18 @@ function ChecklistScore({ studyId, checklistId }) { With the three-category model, most of the current extraction becomes unnecessary: -| Data | Current | Still Needed? | New Approach | -|------|---------|---------------|-------------| -| Study name, description | Full extraction every time | Only for study list | Category 2: StudiesCollection, incremental | -| Checklist answers | Full extraction every time | Only for active checklist | Category 2: per-checklist snapshot, on demand | -| Checklist status, assignee | Full extraction every time | Only for study table | Category 2: StudiesCollection, shallow only | -| Checklist score | Computed during extraction | Only for finalized | Category 3: derived lazily in component | -| PDF list | Full extraction every time | Only for PDF panel | Category 2: per-study, on demand | -| Annotations | Full extraction every time | Only for annotation layer | Category 2: per-checklist, on demand | -| Members | Full extraction every time | Only for member list | Category 2: MembersCollection | -| Meta | Full extraction every time | Only for project header | Category 2: MetaCollection | -| Reconciliation | Full extraction every time | Only for reconcile tab | Category 2: on demand | -| Y.Text content | Extracted as strings | No | Category 1: direct Y.Text binding | +| Data | Current | Still Needed? | New Approach | +| -------------------------- | -------------------------- | ------------------------- | --------------------------------------------- | +| Study name, description | Full extraction every time | Only for study list | Category 2: StudiesCollection, incremental | +| Checklist answers | Full extraction every time | Only for active checklist | Category 2: per-checklist snapshot, on demand | +| Checklist status, assignee | Full extraction every time | Only for study table | Category 2: StudiesCollection, shallow only | +| Checklist score | Computed during extraction | Only for finalized | Category 3: derived lazily in component | +| PDF list | Full extraction every time | Only for PDF panel | Category 2: per-study, on demand | +| Annotations | Full extraction every time | Only for annotation layer | Category 2: per-checklist, on demand | +| Members | Full extraction every time | Only for member list | Category 2: MembersCollection | +| Meta | Full extraction every time | Only for project header | Category 2: MetaCollection | +| Reconciliation | Full extraction every time | Only for reconcile tab | Category 2: on demand | +| Y.Text content | Extracted as strings | No | Category 1: direct Y.Text binding | The entire `extractAnswersFromYMap()` function (120+ lines of type-specific extraction) moves into the checklist type handlers where it belongs. It only runs when building a specific checklist's snapshot, not on every Y.Doc change. @@ -352,7 +357,7 @@ class StudiesCollection { } // Incremental updates via observeDeep - reviewsMap.observeDeep((events) => { + reviewsMap.observeDeep(events => { let changed = false; for (const event of events) { @@ -392,8 +397,7 @@ class StudiesCollection { getSnapshot(): StudySnapshot[] { if (!this.sortedList) { - this.sortedList = [...this.snapshots.values()] - .sort((a, b) => a.createdAt - b.createdAt); + this.sortedList = [...this.snapshots.values()].sort((a, b) => a.createdAt - b.createdAt); } return this.sortedList; // same array ref if nothing changed } @@ -412,6 +416,7 @@ class StudiesCollection { **Key behavior**: `buildStudySnapshot` replaces the per-study portion of `sync.ts:buildStudyFromYMap`. Checklist-type-specific answer extraction moves into the checklist type handlers (AMSTAR2Handler, ROB2Handler, etc.), called by `buildStudySnapshot` only when building that study's snapshot. **observeDeep guarantees** (verified in Y.js source): + - Events are batched within a Y.Doc transaction -- single callback per transaction - `event.path` tells you which subtree changed (computed lazily via parent pointers) - `event.keys` (on YMapEvent) tells you which keys were added/updated/deleted @@ -681,11 +686,7 @@ const diff = compareChecklists(c1.answers, c2.answers, reconciled?.answers); const h1 = useChecklistHandle(studyId, checklist1Id); const h2 = useChecklistHandle(studyId, checklist2Id); const hr = useChecklistHandle(studyId, reconciledId); -const diff = compareChecklists( - h1?.snapshot().answers, - h2?.snapshot().answers, - hr?.snapshot().answers, -); +const diff = compareChecklists(h1?.snapshot().answers, h2?.snapshot().answers, hr?.snapshot().answers); ``` The comparison functions remain unchanged -- they still receive plain objects. The difference is that each handle's `snapshot()` is incrementally maintained and cached, not re-extracted from the full tree on every change. @@ -702,35 +703,35 @@ const { phase, error } = useConnectionState(); ### What Dies -| Current Code | Replacement | -|---|---| -| `sync.ts` (505 lines) | Domain collections with observeDeep. `doSync()`, `buildStudyFromYMap()`, `extractAnswersFromYMap()` all eliminated. Per-entity snapshot building lives in collections and type handlers. | -| `projectStore.projects` map | Eliminated. React reads from collection snapshots via `useSyncExternalStore`. | -| `projectStore.setProjectData()` | Eliminated. No centralized data setter. | -| `JSON.stringify` equality checks | Replaced by reference equality on per-entity snapshots. | -| `buildOpsMap` + `ConnectionOps` | Replaced by typed handles (`ChecklistHandle`, `StudyHandle`, etc.). | -| `requestAnimationFrame` debounce in SyncManager | `observeDeep` fires once per Y.Doc transaction -- natural batching. | -| All `select*` functions in projectStore | Replaced by collection hooks (`useStudies()`, `useMembers()`, etc.). | +| Current Code | Replacement | +| ----------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `sync.ts` (505 lines) | Domain collections with observeDeep. `doSync()`, `buildStudyFromYMap()`, `extractAnswersFromYMap()` all eliminated. Per-entity snapshot building lives in collections and type handlers. | +| `projectStore.projects` map | Eliminated. React reads from collection snapshots via `useSyncExternalStore`. | +| `projectStore.setProjectData()` | Eliminated. No centralized data setter. | +| `JSON.stringify` equality checks | Replaced by reference equality on per-entity snapshots. | +| `buildOpsMap` + `ConnectionOps` | Replaced by typed handles (`ChecklistHandle`, `StudyHandle`, etc.). | +| `requestAnimationFrame` debounce in SyncManager | `observeDeep` fires once per Y.Doc transaction -- natural batching. | +| All `select*` functions in projectStore | Replaced by collection hooks (`useStudies()`, `useMembers()`, etc.). | ### What Survives -| Current Code | Status | -|---|---| -| `connectionReducer.ts` | Stays. Clean state machine, moves into Session layer. | -| `ProjectDoc.ts` (Durable Object) | Stays with minor improvements (alarm-based persistence). | -| y-websocket connection logic | Stays, wrapped in `WebSocketSyncProvider`. | -| y-dexie persistence | Stays, wrapped in `IndexedDBProvider`. | -| Checklist operation factories | Stays, accessed via typed handles instead of flat ops map. | -| Awareness protocol | Stays, wrapped in `AwarenessManager`. | +| Current Code | Status | +| ----------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------- | +| `connectionReducer.ts` | Stays. Clean state machine, moves into Session layer. | +| `ProjectDoc.ts` (Durable Object) | Stays with minor improvements (alarm-based persistence). | +| y-websocket connection logic | Stays, wrapped in `WebSocketSyncProvider`. | +| y-dexie persistence | Stays, wrapped in `IndexedDBProvider`. | +| Checklist operation factories | Stays, accessed via typed handles instead of flat ops map. | +| Awareness protocol | Stays, wrapped in `AwarenessManager`. | | Checklist type handlers (AMSTAR2Handler, ROB2Handler, etc.) | Stays. Answer serialization moves from sync.ts into these handlers, called by collection snapshot builders. | ### What Shrinks -| Current Code | Change | -|---|---| -| `ConnectionPool.ts` (373 lines) | Replaced by `SessionManager` (~50 lines) + `ProjectSession` (~100 lines). Lifecycle, ops, and persistence concerns separated. | -| `projectStore.ts` (274 lines) | Shrinks to ~40 lines. Only holds `activeProjectId`, `connections` (state machine), and `projectStats` (localStorage-persisted). No more `projects` map or selectors. | -| `connection.ts` (286 lines) | Stays roughly the same size but encapsulated as `WebSocketSyncProvider`. | +| Current Code | Change | +| ------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `ConnectionPool.ts` (373 lines) | Replaced by `SessionManager` (~50 lines) + `ProjectSession` (~100 lines). Lifecycle, ops, and persistence concerns separated. | +| `projectStore.ts` (274 lines) | Shrinks to ~40 lines. Only holds `activeProjectId`, `connections` (state machine), and `projectStats` (localStorage-persisted). No more `projects` map or selectors. | +| `connection.ts` (286 lines) | Stays roughly the same size but encapsulated as `WebSocketSyncProvider`. | --- @@ -815,10 +816,10 @@ Since this work happens on a feature branch, Phase 6 is the gate before merge. E These files should be fully removable after Phase 4: -| File | Condition for Deletion | -|---|---| -| `src/primitives/useProject/sync.ts` | No imports remain. `SyncManager` type not referenced. | -| `src/project/ConnectionPool.ts` | Replaced by `SessionManager`. No imports remain. | +| File | Condition for Deletion | +| ----------------------------------------- | ------------------------------------------------------------ | +| `src/primitives/useProject/sync.ts` | No imports remain. `SyncManager` type not referenced. | +| `src/project/ConnectionPool.ts` | Replaced by `SessionManager`. No imports remain. | | `src/primitives/useProject/connection.ts` | Logic moved into `WebSocketSyncProvider`. No imports remain. | Verify with: `grep -r "from.*sync" src/ --include="*.ts" --include="*.tsx"` (and equivalent for each file). @@ -835,6 +836,7 @@ After all consumers migrate, these exports become unused: - `setPendingProjectData`, `getPendingProjectData` What remains in `projectStore.ts`: + - `activeProjectId` + `setActiveProject` - `connections` map + `dispatchConnectionEvent` (state machine) - `projectStats` + `selectProjectStats` (localStorage-persisted, used by dashboard) @@ -858,19 +860,19 @@ Verify with: `pnpm --filter landing build` (tree-shaking will flag unused import After migration, this directory's contents change significantly: -| Current | After | -|---|---| -| `sync.ts` | Deleted | -| `connection.ts` | Deleted (logic in `WebSocketSyncProvider`) | -| `studies.ts` | Stays, but accessed via `StudyHandle` instead of flat ops | -| `checklists/index.ts` | Stays, but accessed via `ChecklistHandle` | -| `checklists/AMSTAR2Handler.ts` | Stays, gains `readAnswers()` from old sync.ts extraction | -| `checklists/ROB2Handler.ts` | Same | -| `checklists/ROBINSIHandler.ts` | Same | -| `pdfs.ts` | Stays, accessed via handle | -| `reconciliation.ts` | Stays, accessed via handle | -| `annotations.ts` | Stays, accessed via handle | -| `outcomes.ts` | Stays, accessed via handle | +| Current | After | +| ------------------------------ | --------------------------------------------------------- | +| `sync.ts` | Deleted | +| `connection.ts` | Deleted (logic in `WebSocketSyncProvider`) | +| `studies.ts` | Stays, but accessed via `StudyHandle` instead of flat ops | +| `checklists/index.ts` | Stays, but accessed via `ChecklistHandle` | +| `checklists/AMSTAR2Handler.ts` | Stays, gains `readAnswers()` from old sync.ts extraction | +| `checklists/ROB2Handler.ts` | Same | +| `checklists/ROBINSIHandler.ts` | Same | +| `pdfs.ts` | Stays, accessed via handle | +| `reconciliation.ts` | Stays, accessed via handle | +| `annotations.ts` | Stays, accessed via handle | +| `outcomes.ts` | Stays, accessed via handle | Consider reorganizing the directory to reflect the new structure: @@ -909,16 +911,16 @@ src/project/ #### 6f: Update Documentation -| Document | Changes Needed | -|---|---| -| `packages/docs/guides/state-management.md` | Remove projectStore data flow, document collection + handle pattern | -| `packages/docs/guides/components.md` | Update data access examples | -| `packages/docs/audits/project-sync-behavior-spec.md` | Rewrite to reflect new architecture | -| `.claude/yjs-sync.mdc` | Update sync pipeline description, remove SyncManager references | -| `.claude/durable-objects.mdc` | Add alarm-based persistence, incremental updates | -| `.claude/reconciliation.mdc` | Update to reference typed handles instead of connectionPool ops | -| `.claude/checklist-operations.mdc` | Update to reference ChecklistHandle API | -| `CLAUDE.md` | Update if any top-level patterns changed | +| Document | Changes Needed | +| ---------------------------------------------------- | ------------------------------------------------------------------- | +| `packages/docs/guides/state-management.md` | Remove projectStore data flow, document collection + handle pattern | +| `packages/docs/guides/components.md` | Update data access examples | +| `packages/docs/audits/project-sync-behavior-spec.md` | Rewrite to reflect new architecture | +| `.claude/yjs-sync.mdc` | Update sync pipeline description, remove SyncManager references | +| `.claude/durable-objects.mdc` | Add alarm-based persistence, incremental updates | +| `.claude/reconciliation.mdc` | Update to reference typed handles instead of connectionPool ops | +| `.claude/checklist-operations.mdc` | Update to reference ChecklistHandle API | +| `CLAUDE.md` | Update if any top-level patterns changed | #### 6g: Pre-Merge Checklist @@ -943,15 +945,15 @@ Before opening the PR to merge the feature branch: ## Risk Assessment -| Risk | Severity | Mitigation | -|---|---|---| -| Regression in real-time sync | High | Dual-path during phases 1-3. Both systems read from same Y.Doc. E2E tests catch regressions early. | -| observeDeep performance | Medium | Profile in phase 1 with large projects (50+ studies). Lazy path computation is the main cost, but eliminates full tree walk. Net positive expected. Benchmark before committing to phase 2. | -| Snapshot staleness | Low | observeDeep fires synchronously at end of Y.Doc transaction. Snapshots are always consistent with current Y.Doc state. No async gap between mutation and observation. | -| Reconciliation comparison breaks | High | Comparison functions receive the same plain objects as today. Only the source changes (collection snapshot vs sync.ts extraction). E2E reconciliation workflows are the primary gate. | -| observeDeep event.path assumptions | Medium | Must handle top-level changes (study added/removed) where path.length === 0 separately from nested changes. Verified in Y.js source: YMapEvent.keys provides add/update/delete info at each level. Must extract event data inside callback (stale after return). | -| Migration stalls mid-way | Medium | Each phase is independently valuable. Phase 2 alone (incremental snapshots) eliminates the biggest performance problem. Phase 3 alone (typed handles) eliminates type erasure. Neither depends on the other being complete. | -| Memory overhead from snapshot cache | Low | Current system already holds the full extracted state in Zustand. New system holds per-entity snapshots in collection Maps. Total memory roughly equivalent, possibly lower since unchanged entities share references. | +| Risk | Severity | Mitigation | +| ----------------------------------- | -------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Regression in real-time sync | High | Dual-path during phases 1-3. Both systems read from same Y.Doc. E2E tests catch regressions early. | +| observeDeep performance | Medium | Profile in phase 1 with large projects (50+ studies). Lazy path computation is the main cost, but eliminates full tree walk. Net positive expected. Benchmark before committing to phase 2. | +| Snapshot staleness | Low | observeDeep fires synchronously at end of Y.Doc transaction. Snapshots are always consistent with current Y.Doc state. No async gap between mutation and observation. | +| Reconciliation comparison breaks | High | Comparison functions receive the same plain objects as today. Only the source changes (collection snapshot vs sync.ts extraction). E2E reconciliation workflows are the primary gate. | +| observeDeep event.path assumptions | Medium | Must handle top-level changes (study added/removed) where path.length === 0 separately from nested changes. Verified in Y.js source: YMapEvent.keys provides add/update/delete info at each level. Must extract event data inside callback (stale after return). | +| Migration stalls mid-way | Medium | Each phase is independently valuable. Phase 2 alone (incremental snapshots) eliminates the biggest performance problem. Phase 3 alone (typed handles) eliminates type erasure. Neither depends on the other being complete. | +| Memory overhead from snapshot cache | Low | Current system already holds the full extracted state in Zustand. New system holds per-entity snapshots in collection Maps. Total memory roughly equivalent, possibly lower since unchanged entities share references. | --- @@ -959,28 +961,28 @@ Before opening the PR to merge the feature branch: ### Display Components (10) -- Migrate in Phase 2 -| Component | Current Store Access | New Access | -|---|---|---| -| ProjectView | selectStudies, selectMeta, selectConnectionPhase | useStudies, useMeta, useConnectionState | -| OverviewTab | selectStudies, selectMembers | useStudies, useMembers | -| AllStudiesTab | selectStudies, selectMembers, selectConnectionPhase | useStudies, useMembers, useConnectionState | -| TodoTab | selectStudies, selectMembers, selectConnectionPhase | useStudies, useMembers, useConnectionState | -| ReconcileTab | selectStudies, selectMeta | useStudies, useMeta | -| CompletedTab | selectStudies, meta | useStudies, useMeta | -| ProjectCard | selectProjectStats | Derived from useStudies | -| TodoStudyRow | meta (outcomes) | useMeta | -| AssignReviewersModal | selectMembers, selectStudies | useMembers, useStudies | -| PreviousReviewersView | getChecklistData (read-only) | useChecklistHandle.snapshot() | +| Component | Current Store Access | New Access | +| --------------------- | --------------------------------------------------- | ------------------------------------------ | +| ProjectView | selectStudies, selectMeta, selectConnectionPhase | useStudies, useMeta, useConnectionState | +| OverviewTab | selectStudies, selectMembers | useStudies, useMembers | +| AllStudiesTab | selectStudies, selectMembers, selectConnectionPhase | useStudies, useMembers, useConnectionState | +| TodoTab | selectStudies, selectMembers, selectConnectionPhase | useStudies, useMembers, useConnectionState | +| ReconcileTab | selectStudies, selectMeta | useStudies, useMeta | +| CompletedTab | selectStudies, meta | useStudies, useMeta | +| ProjectCard | selectProjectStats | Derived from useStudies | +| TodoStudyRow | meta (outcomes) | useMeta | +| AssignReviewersModal | selectMembers, selectStudies | useMembers, useStudies | +| PreviousReviewersView | getChecklistData (read-only) | useChecklistHandle.snapshot() | ### Editor Components (5 primary) -- Migrate in Phase 3 -| Component | Current Access | New Access | -|---|---|---| -| ChecklistYjsWrapper | connectionPool.get(): getChecklistData, updateChecklistAnswer, getQuestionNote, getRobinsText, getRob2Text, addAnnotation, updateAnnotation, deleteAnnotation | useChecklistHandle: snapshot(), setAnswer(), textRef(). Annotation handle for PDF operations. | -| ReconciliationWrapper | connectionPool.get(): getChecklistData (x3), updateChecklistAnswer, saveReconciliationProgress, getReconciliationProgress | useChecklistHandle (x3): snapshot(), setAnswer(). useReconciliationHandle: save(), load(). | -| ReconciliationEngine | Operations passed via props | Handles passed via props | -| ROB2/ROBINS-I/AMSTAR2 Adapters | updateChecklistAnswer, getChecklistData | handle.setAnswer(), handle.snapshot() | -| OutcomeManager | meta (outcomes) | useMeta, outcome handle | +| Component | Current Access | New Access | +| ------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------- | +| ChecklistYjsWrapper | connectionPool.get(): getChecklistData, updateChecklistAnswer, getQuestionNote, getRobinsText, getRob2Text, addAnnotation, updateAnnotation, deleteAnnotation | useChecklistHandle: snapshot(), setAnswer(), textRef(). Annotation handle for PDF operations. | +| ReconciliationWrapper | connectionPool.get(): getChecklistData (x3), updateChecklistAnswer, saveReconciliationProgress, getReconciliationProgress | useChecklistHandle (x3): snapshot(), setAnswer(). useReconciliationHandle: save(), load(). | +| ReconciliationEngine | Operations passed via props | Handles passed via props | +| ROB2/ROBINS-I/AMSTAR2 Adapters | updateChecklistAnswer, getChecklistData | handle.setAnswer(), handle.snapshot() | +| OutcomeManager | meta (outcomes) | useMeta, outcome handle | ### Prop-Driven Components (20+) -- No Migration Needed @@ -992,11 +994,11 @@ All components reading `selectConnectionPhase` switch to `useConnectionState()`. ### Dependency Counts -| Import | File Count | -|---|---| -| `useProjectStore` + selectors | 21 files | -| `connectionPool` | 15 files | -| `sync.ts` (SyncManager) | 1 file (ConnectionPool only) | -| `connectionReducer` | 2 files (projectStore, ConnectionPool) | +| Import | File Count | +| ----------------------------- | -------------------------------------- | +| `useProjectStore` + selectors | 21 files | +| `connectionPool` | 15 files | +| `sync.ts` (SyncManager) | 1 file (ConnectionPool only) | +| `connectionReducer` | 2 files (projectStore, ConnectionPool) | No circular dependencies. `sync.ts` is only imported by `ConnectionPool`, making it the easiest to remove once collections replace its role. diff --git a/packages/docs/audits/yjs-sync-pipeline-redesign-b.md b/packages/docs/audits/yjs-sync-pipeline-redesign-b.md index 00c3abc56..2fde5e851 100644 --- a/packages/docs/audits/yjs-sync-pipeline-redesign-b.md +++ b/packages/docs/audits/yjs-sync-pipeline-redesign-b.md @@ -52,13 +52,13 @@ But the proposed solution -- domain collections, typed handles, a three-layer ar Instead of replacing the architecture, fix the four specific bottlenecks: -| Problem | Fix | Files Touched | -|---------|-----|---------------| -| Full-tree extraction on every change | Scoped `observeDeep` per top-level Y.Map | `sync.ts`, `ConnectionPool.ts` | -| O(n) rebuild within `doSync` | Per-study snapshot cache with dirty tracking | `sync.ts` | -| `JSON.stringify` equality | Reference equality via cached snapshots | `projectStore.ts` | -| Type erasure in `buildOpsMap` | Delete `buildOpsMap`, expose typed ops directly | `ConnectionPool.ts`, consumers of `.get()` | -| Unnecessary indirection layers | Delete thin action wrappers, delete handles | Action modules, handle files | +| Problem | Fix | Files Touched | +| ------------------------------------ | ----------------------------------------------- | ------------------------------------------ | +| Full-tree extraction on every change | Scoped `observeDeep` per top-level Y.Map | `sync.ts`, `ConnectionPool.ts` | +| O(n) rebuild within `doSync` | Per-study snapshot cache with dirty tracking | `sync.ts` | +| `JSON.stringify` equality | Reference equality via cached snapshots | `projectStore.ts` | +| Type erasure in `buildOpsMap` | Delete `buildOpsMap`, expose typed ops directly | `ConnectionPool.ts`, consumers of `.get()` | +| Unnecessary indirection layers | Delete thin action wrappers, delete handles | Action modules, handle files | Total: ~5 files significantly changed, ~5 thin files deleted. Zero component API changes for the performance fixes. Typed ops require updating consumers of `connectionPool.get()` (15 files, mechanical find-and-replace). @@ -110,7 +110,7 @@ export function createSyncManager(projectId: string, getYDoc: () => Y.Doc | null }; reviewsMap.observeDeep(onReviews); - membersMap.observe(onMembers); // flat, no need for observeDeep + membersMap.observe(onMembers); // flat, no need for observeDeep metaMap.observeDeep(onMeta); return [ @@ -197,7 +197,7 @@ function handleReviewsEvents(events: Y.YEvent[]): void { if (!reviewsMap) return; const dirtyStudyIds = new Set(); - let structuralChange = false; // study added or removed + let structuralChange = false; // study added or removed for (const event of events) { if (event.path.length === 0 && 'keys' in event) { @@ -234,8 +234,7 @@ function handleReviewsEvents(events: Y.YEvent[]): void { // Rebuild sorted array only if something changed if (dirtyStudyIds.size > 0 || structuralChange) { - sortedStudies = [...studyCache.values()] - .sort((a, b) => (a.createdAt || 0) - (b.createdAt || 0)); + sortedStudies = [...studyCache.values()].sort((a, b) => (a.createdAt || 0) - (b.createdAt || 0)); } } @@ -248,8 +247,7 @@ function syncStudies(ydoc: Y.Doc): StudyInfo[] { const studyData = ymap.toJSON ? ymap.toJSON() : {}; studyCache.set(studyId, buildStudyFromYMap(studyId, studyData, ymap)); } - sortedStudies = [...studyCache.values()] - .sort((a, b) => (a.createdAt || 0) - (b.createdAt || 0)); + sortedStudies = [...studyCache.values()].sort((a, b) => (a.createdAt || 0) - (b.createdAt || 0)); } return sortedStudies; } @@ -271,6 +269,7 @@ More precisely: ### Reference equality follows naturally When study-123's checklist answer changes: + - `buildStudyFromYMap` creates a new `StudyInfo` object for study-123 - `studyCache.get('study-456')` returns the exact same object reference as before - `sortedStudies` is a new array (unavoidable), but the individual study objects inside it have stable references for unchanged studies @@ -283,6 +282,7 @@ This is the same granularity the redesign RFC claims, without collections, witho The `sortedStudies` array is a new reference on every change. Components that call `selectStudies` and iterate the array will re-render. This is the same as today. Two mitigations, both optional: + 1. Components that render individual studies should use `selectStudy(state, projectId, studyId)` instead of mapping over `selectStudies`. This already works with the current selector. 2. If array stability becomes a measured problem, the store setter can check whether the array contents actually changed (shallow element comparison, not `JSON.stringify`). @@ -441,15 +441,15 @@ Component ### Which action modules to delete -| Module | Lines | Real Logic? | Verdict | -|--------|-------|-------------|---------| -| `checklists.ts` | 82 | No. Try/catch + toast only. | Delete. | -| `outcomes.ts` | ~40 | No. Same pattern. | Delete. | -| `reconciliation.ts` | ~30 | No. Same pattern. | Delete. | -| `members.ts` | ~30 | No. Same pattern. | Delete. | -| `project.ts` | ~30 | No. Forwards to studyOps rename/description. | Delete. | -| `pdfs.ts` | ~60 | Minor. Wraps upload + R2 delete. | Keep or inline. | -| `studies.ts` | 443 | Yes. Batch import, PDF upload, Google Drive import, metadata extraction. | Keep. Real domain logic. | +| Module | Lines | Real Logic? | Verdict | +| ------------------- | ----- | ------------------------------------------------------------------------ | ------------------------ | +| `checklists.ts` | 82 | No. Try/catch + toast only. | Delete. | +| `outcomes.ts` | ~40 | No. Same pattern. | Delete. | +| `reconciliation.ts` | ~30 | No. Same pattern. | Delete. | +| `members.ts` | ~30 | No. Same pattern. | Delete. | +| `project.ts` | ~30 | No. Forwards to studyOps rename/description. | Delete. | +| `pdfs.ts` | ~60 | Minor. Wraps upload + R2 delete. | Keep or inline. | +| `studies.ts` | 443 | Yes. Batch import, PDF upload, Google Drive import, metadata extraction. | Keep. Real domain logic. | `studies.ts` is the only action module with logic worth keeping. The rest are forwarding functions. Their error handling (try/catch + toast) can either move to the component or to a thin shared utility if the pattern is common enough. @@ -461,20 +461,20 @@ The staged files on the current branch (`handles/ChecklistHandle.ts`, `handles/A ## What Does Not Change -| Component | Status | -|-----------|--------| -| `ConnectionPool.ts` | Stays. Loses `buildOpsMap` and `opsRegistry` (~60 lines). Gains `getOps()` returning typed interface (~15 lines). Net reduction. | -| `connectionReducer.ts` | Unchanged. Clean state machine. | -| `connection.ts` | Unchanged. WebSocket management. | -| `projectStore.ts` selectors | Unchanged. `selectStudies`, `selectStudy`, `selectMembers`, `selectMeta`, `selectConnectionPhase` all continue to work. | -| `projectStore.ts` shape | Unchanged. `projects` map, `connections`, `projectStats` all stay. | -| Component data access (reads) | Unchanged. Components still call `useProjectStore(s => selectStudies(s, projectId))`. | -| Operation factories | Unchanged. `createChecklistOperations`, etc. remain the logic layer. | -| Checklist type handlers | Unchanged. `AMSTAR2Handler`, `ROB2Handler`, `ROBINSIHandler` remain. | -| `extractAnswersFromYMap` | Stays in `sync.ts`. Called per-study now instead of for all studies, but the function itself does not change. | -| Y.Doc structure | Unchanged. Same maps, same nesting. | -| ProjectDoc (Durable Object) | Unchanged. Server-side improvements (alarm-based persistence, incremental updates) are orthogonal and can be done independently. | -| Zustand as the React integration | Stays. No `useSyncExternalStore`, no React context for session. Components subscribe to the same store with the same selectors. | +| Component | Status | +| -------------------------------- | -------------------------------------------------------------------------------------------------------------------------------- | +| `ConnectionPool.ts` | Stays. Loses `buildOpsMap` and `opsRegistry` (~60 lines). Gains `getOps()` returning typed interface (~15 lines). Net reduction. | +| `connectionReducer.ts` | Unchanged. Clean state machine. | +| `connection.ts` | Unchanged. WebSocket management. | +| `projectStore.ts` selectors | Unchanged. `selectStudies`, `selectStudy`, `selectMembers`, `selectMeta`, `selectConnectionPhase` all continue to work. | +| `projectStore.ts` shape | Unchanged. `projects` map, `connections`, `projectStats` all stay. | +| Component data access (reads) | Unchanged. Components still call `useProjectStore(s => selectStudies(s, projectId))`. | +| Operation factories | Unchanged. `createChecklistOperations`, etc. remain the logic layer. | +| Checklist type handlers | Unchanged. `AMSTAR2Handler`, `ROB2Handler`, `ROBINSIHandler` remain. | +| `extractAnswersFromYMap` | Stays in `sync.ts`. Called per-study now instead of for all studies, but the function itself does not change. | +| Y.Doc structure | Unchanged. Same maps, same nesting. | +| ProjectDoc (Durable Object) | Unchanged. Server-side improvements (alarm-based persistence, incremental updates) are orthogonal and can be done independently. | +| Zustand as the React integration | Stays. No `useSyncExternalStore`, no React context for session. Components subscribe to the same store with the same selectors. | --- @@ -509,6 +509,7 @@ Three phases, each independently shippable. Each phase is a single PR. No coexis **Files changed**: `sync.ts` (bulk of changes), `ConnectionPool.ts` (~5 lines), `projectStore.ts` (~6 lines). **Validation**: + - `pnpm --filter landing test` (unit tests) - `pnpm --filter landing test:browser` (E2E) - Manual: open a 20+ study project, edit a checklist answer, verify via React DevTools that only the edited study's component subtree re-renders @@ -540,6 +541,7 @@ Three phases, each independently shippable. Each phase is a single PR. No coexis **Files changed**: `ConnectionPool.ts`, ~15 consumer files (action modules + components that access ops directly). **Validation**: + - `pnpm typecheck` (primary gate -- type errors reveal any missed call site) - `pnpm --filter landing test` - `pnpm --filter landing test:browser` @@ -559,10 +561,12 @@ Three phases, each independently shippable. Each phase is a single PR. No coexis 3. Keep `studies.ts` -- update it to use `TypedProjectOps`. 4. If the try/catch + toast pattern repeats at 3+ call sites for the same operation, extract a utility: + ```typescript function withToast(fn: () => T, errorTitle: string): T | null { - try { return fn(); } - catch (err) { + try { + return fn(); + } catch (err) { console.error(err); showToast.error(errorTitle, (err as Error).message); return null; @@ -575,6 +579,7 @@ Three phases, each independently shippable. Each phase is a single PR. No coexis **Files changed**: 5 action modules deleted, ~10 consumer files updated. **Validation**: + - `pnpm typecheck` - `pnpm --filter landing test` - `pnpm --filter landing test:browser` @@ -586,27 +591,27 @@ Three phases, each independently shippable. Each phase is a single PR. No coexis ## Risk Assessment -| Risk | Severity | Mitigation | -|------|----------|------------| -| `observeDeep` event path incorrect for some Y.Doc structure | Medium | Unit test: create Y.Doc, mutate a nested key, assert `event.path[0]` is the study ID. Y.js source confirms this behavior for nested Y.Maps. | -| Study cache gets stale (snapshot diverges from Y.Doc) | Medium | Cache is populated inside the `observeDeep` callback, which fires synchronously at end of Y.Doc transaction. No async gap. Also: `syncFromYDocImmediate()` can clear the cache and rebuild from scratch as a safety valve. | -| `requestAnimationFrame` batching drops events | Low | `observeDeep` fires with accumulated events per transaction. The RAF deduplication only affects the store write, not the event collection. Dirty study IDs accumulate across RAF frames via the study cache (which is updated synchronously in the callback). | -| Consumers missed during `get()` to `getOps()` migration | None | `pnpm typecheck` catches every call site. `get()` is deleted, so any unconverted call is a compile error. | -| Performance regression from `observeDeep` overhead | Low | `observeDeep` is how Y.js recommends observing nested changes. The overhead is lazy path computation, which is far cheaper than the current full-tree `toJSON()` + `JSON.stringify` comparison. Profile in Phase 1 validation. | +| Risk | Severity | Mitigation | +| ----------------------------------------------------------- | -------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `observeDeep` event path incorrect for some Y.Doc structure | Medium | Unit test: create Y.Doc, mutate a nested key, assert `event.path[0]` is the study ID. Y.js source confirms this behavior for nested Y.Maps. | +| Study cache gets stale (snapshot diverges from Y.Doc) | Medium | Cache is populated inside the `observeDeep` callback, which fires synchronously at end of Y.Doc transaction. No async gap. Also: `syncFromYDocImmediate()` can clear the cache and rebuild from scratch as a safety valve. | +| `requestAnimationFrame` batching drops events | Low | `observeDeep` fires with accumulated events per transaction. The RAF deduplication only affects the store write, not the event collection. Dirty study IDs accumulate across RAF frames via the study cache (which is updated synchronously in the callback). | +| Consumers missed during `get()` to `getOps()` migration | None | `pnpm typecheck` catches every call site. `get()` is deleted, so any unconverted call is a compile error. | +| Performance regression from `observeDeep` overhead | Low | `observeDeep` is how Y.js recommends observing nested changes. The overhead is lazy path computation, which is far cheaper than the current full-tree `toJSON()` + `JSON.stringify` comparison. Profile in Phase 1 validation. | --- ## Comparison with Redesign RFC -| | Redesign RFC (A) | Targeted Fixes (B) | -|---|---|---| -| Files significantly changed | 36+ across 6 phases | ~8 across 3 phases | -| Component API changes | 73 components migrate to new hooks | Zero for Phase 1. 15 files for Phase 2 (mechanical namespace change). | -| New abstractions introduced | Collections, Handles, Sessions, SessionManager, Providers, SessionContext | None | -| Abstractions removed | sync.ts, projectStore selectors, ConnectionPool, action modules | buildOpsMap, opsRegistry, ConnectionOps type, thin action modules | -| Coexistence period | Months (dual-path reads through Phases 1-3) | None. Each phase replaces in-place. | -| Performance improvement | O(1 study) per change | O(1 study) per change | -| Type safety improvement | Handles with `Record` snapshots | Typed ops with full `ChecklistOperations` etc. signatures | -| Risk of stalling mid-migration | High (Phase 3 is indirection until Phase 4 absorbs factories) | Low (each phase is independently complete) | -| Total lines added (estimated) | 1500+ (new abstractions) | ~200 (cache logic in sync.ts, typed interface) | -| Total lines removed (estimated) | 1200 (after full 6-phase migration) | ~400 (buildOpsMap, thin wrappers, JSON.stringify checks) | +| | Redesign RFC (A) | Targeted Fixes (B) | +| ------------------------------- | ------------------------------------------------------------------------- | --------------------------------------------------------------------- | +| Files significantly changed | 36+ across 6 phases | ~8 across 3 phases | +| Component API changes | 73 components migrate to new hooks | Zero for Phase 1. 15 files for Phase 2 (mechanical namespace change). | +| New abstractions introduced | Collections, Handles, Sessions, SessionManager, Providers, SessionContext | None | +| Abstractions removed | sync.ts, projectStore selectors, ConnectionPool, action modules | buildOpsMap, opsRegistry, ConnectionOps type, thin action modules | +| Coexistence period | Months (dual-path reads through Phases 1-3) | None. Each phase replaces in-place. | +| Performance improvement | O(1 study) per change | O(1 study) per change | +| Type safety improvement | Handles with `Record` snapshots | Typed ops with full `ChecklistOperations` etc. signatures | +| Risk of stalling mid-migration | High (Phase 3 is indirection until Phase 4 absorbs factories) | Low (each phase is independently complete) | +| Total lines added (estimated) | 1500+ (new abstractions) | ~200 (cache logic in sync.ts, typed interface) | +| Total lines removed (estimated) | 1200 (after full 6-phase migration) | ~400 (buildOpsMap, thin wrappers, JSON.stringify checks) | diff --git a/packages/landing/e2e/auth-flow.spec.ts b/packages/landing/e2e/auth-flow.spec.ts index f88925608..7bd54765b 100644 --- a/packages/landing/e2e/auth-flow.spec.ts +++ b/packages/landing/e2e/auth-flow.spec.ts @@ -71,11 +71,15 @@ test.describe('Auth flows', () => { await page.getByRole('button', { name: 'Next' }).click(); // Step 2: Institution - skip - await expect(page.getByRole('heading', { name: 'Institution Details' })).toBeVisible({ timeout: 5_000 }); + await expect(page.getByRole('heading', { name: 'Institution Details' })).toBeVisible({ + timeout: 5_000, + }); await page.getByRole('button', { name: /Skip for now/i }).click(); // Step 3: Role - select Researcher - await expect(page.getByRole('heading', { name: /What best describes you/i })).toBeVisible({ timeout: 5_000 }); + await expect(page.getByRole('heading', { name: /What best describes you/i })).toBeVisible({ + timeout: 5_000, + }); await page.getByText('Researcher').click(); await page.getByRole('button', { name: /Finish Setup/i }).click(); @@ -132,7 +136,9 @@ test.describe('Auth flows', () => { // Navigate to reset password page await page.goto('/reset-password'); - await expect(page.getByRole('heading', { name: 'Reset Password' })).toBeVisible({ timeout: 10_000 }); + await expect(page.getByRole('heading', { name: 'Reset Password' })).toBeVisible({ + timeout: 10_000, + }); // Request password reset const resetEmailField = page.locator('#email-input'); @@ -154,7 +160,9 @@ test.describe('Auth flows', () => { // Navigate to the frontend reset page with the token await page.goto(`/reset-password?token=${token}`); - await expect(page.getByRole('heading', { name: 'Set New Password' })).toBeVisible({ timeout: 10_000 }); + await expect(page.getByRole('heading', { name: 'Set New Password' })).toBeVisible({ + timeout: 10_000, + }); // Fill and submit new password const newPwField = page.locator('#new-password-input'); diff --git a/packages/landing/e2e/rob2-workflow.spec.ts b/packages/landing/e2e/rob2-workflow.spec.ts index c89fce4c7..e2d6edf6a 100644 --- a/packages/landing/e2e/rob2-workflow.spec.ts +++ b/packages/landing/e2e/rob2-workflow.spec.ts @@ -175,14 +175,18 @@ test('Dual-Reviewer ROB2 Workflow', async ({ context, page }) => { await page.waitForTimeout(1000); } - // For direction pages where reviewers didn't set a value, "Use This" // copies null which doesn't count as answered. Pick "NA" in the Final // Direction panel if visible. const finalDirectionHeading = page.getByText('Final Direction'); if (await finalDirectionHeading.isVisible().catch(() => false)) { const naLabel = page.locator('label').filter({ hasText: /^NA$/ }); - if (await naLabel.first().isVisible().catch(() => false)) { + if ( + await naLabel + .first() + .isVisible() + .catch(() => false) + ) { await naLabel.first().click(); await page.waitForTimeout(300); } diff --git a/packages/landing/src/components/auth/RoleSelector.tsx b/packages/landing/src/components/auth/RoleSelector.tsx index b6932c533..cfa163d5d 100644 --- a/packages/landing/src/components/auth/RoleSelector.tsx +++ b/packages/landing/src/components/auth/RoleSelector.tsx @@ -36,12 +36,10 @@ export function getRoleLabel(roleId: string): string { return ROLES.find(r => r.id === roleId)?.label || roleId; } - interface RoleSelectorProps { selectedRole: string; onSelect: (roleId: string) => void; } - export function RoleSelector({ selectedRole, onSelect }: RoleSelectorProps) { return ( diff --git a/packages/landing/src/components/auth/StrengthIndicator.tsx b/packages/landing/src/components/auth/StrengthIndicator.tsx index 0b2c59c93..491c67f54 100644 --- a/packages/landing/src/components/auth/StrengthIndicator.tsx +++ b/packages/landing/src/components/auth/StrengthIndicator.tsx @@ -29,12 +29,10 @@ const requirementsList = [ }, ]; - interface StrengthIndicatorProps { password: string; onUnmet?: (errors: string[]) => void; } - export function StrengthIndicator({ password, onUnmet }: StrengthIndicatorProps) { const strength = useMemo(() => { diff --git a/packages/landing/src/components/charts/AMSTARDistribution.tsx b/packages/landing/src/components/charts/AMSTARDistribution.tsx index c403713c4..15ffe9031 100644 --- a/packages/landing/src/components/charts/AMSTARDistribution.tsx +++ b/packages/landing/src/components/charts/AMSTARDistribution.tsx @@ -45,11 +45,10 @@ export function AMSTARDistribution({ greyscale = false, ref, }: AMSTARDistributionProps) { - const svgRef = useRef(null); + const svgRef = useRef(null); const containerRef = useRef(null); const [containerSize, setContainerSize] = useState({ width: 900, height: 600 }); - useImperativeHandle(ref, () => svgRef.current as SVGSVGElement, []); // ResizeObserver for responsive sizing diff --git a/packages/landing/src/components/charts/AMSTARRobvis.tsx b/packages/landing/src/components/charts/AMSTARRobvis.tsx index e936219ad..b9775b083 100644 --- a/packages/landing/src/components/charts/AMSTARRobvis.tsx +++ b/packages/landing/src/components/charts/AMSTARRobvis.tsx @@ -46,13 +46,13 @@ export function AMSTARRobvis({ greyscale = false, ref, }: AMSTARRobvisProps) { - const svgRef = useRef(null); + const svgRef = useRef(null); const containerRef = useRef(null); const [containerSize, setContainerSize] = useState({ width: 800, height: 500 }); const [dynamicMarginLeft, setDynamicMarginLeft] = useState(200); // Expose SVG ref to parent for export - + useImperativeHandle(ref, () => svgRef.current as SVGSVGElement, []); // ResizeObserver for responsive sizing diff --git a/packages/landing/src/components/checklist/ChecklistYjsWrapper.tsx b/packages/landing/src/components/checklist/ChecklistYjsWrapper.tsx index 1473950c3..1c57471f3 100644 --- a/packages/landing/src/components/checklist/ChecklistYjsWrapper.tsx +++ b/packages/landing/src/components/checklist/ChecklistYjsWrapper.tsx @@ -91,11 +91,7 @@ export function ChecklistYjsWrapper({ projectId, studyId, checklistId }: Checkli getRob2Text, } = ops.checklist; const { addPdfToStudy } = ops.pdf; - const { - addAnnotation, - updateAnnotation, - deleteAnnotation, - } = ops.annotation; + const { addAnnotation, updateAnnotation, deleteAnnotation } = ops.annotation; const connectionState = useProjectStore(s => selectConnectionPhase(s, projectId)); @@ -234,7 +230,7 @@ export function ChecklistYjsWrapper({ projectId, studyId, checklistId }: Checkli name: currentStudy?.name || 'Checklist', reviewerName: '', createdAt: currentChecklist.createdAt, - ...(data.answers as Record ?? {}), + ...((data.answers as Record) ?? {}), }; }, [currentChecklist, currentStudy, getChecklistData, studyId, checklistId]); @@ -426,7 +422,9 @@ export function ChecklistYjsWrapper({ projectId, studyId, checklistId }: Checkli return (
- {connectionState.phase === 'connecting' || pdfLoading ? 'Loading...' : 'Checklist not found'} + {connectionState.phase === 'connecting' || pdfLoading ? + 'Loading...' + : 'Checklist not found'}
); @@ -447,9 +445,7 @@ export function ChecklistYjsWrapper({ projectId, studyId, checklistId }: Checkli pdfs={studyPdfs} selectedPdfId={selectedPdfId} onPdfSelect={handlePdfSelect} - getQuestionNote={(questionKey: string) => - getQuestionNote(studyId, checklistId, questionKey) - } + getQuestionNote={(questionKey: string) => getQuestionNote(studyId, checklistId, questionKey)} getRobinsText={(sectionKey: string, fieldKey: string, questionKey?: string) => getRobinsText(studyId, checklistId, sectionKey, fieldKey, questionKey) } diff --git a/packages/landing/src/components/checklist/ROBINSIChecklist/checklist-compare.ts b/packages/landing/src/components/checklist/ROBINSIChecklist/checklist-compare.ts index 4450b3501..c49b850a3 100644 --- a/packages/landing/src/components/checklist/ROBINSIChecklist/checklist-compare.ts +++ b/packages/landing/src/components/checklist/ROBINSIChecklist/checklist-compare.ts @@ -576,6 +576,8 @@ export function getDomainDef( * Get the domain name/title */ export function getDomainName(domainKey: string): string { - return (ROBINS_I_CHECKLIST[domainKey as keyof typeof ROBINS_I_CHECKLIST] as { name?: string }) - ?.name || domainKey; + return ( + (ROBINS_I_CHECKLIST[domainKey as keyof typeof ROBINS_I_CHECKLIST] as { name?: string })?.name || + domainKey + ); } diff --git a/packages/landing/src/components/checklist/SplitPanelControls.tsx b/packages/landing/src/components/checklist/SplitPanelControls.tsx index f877cf97d..31727a8bd 100644 --- a/packages/landing/src/components/checklist/SplitPanelControls.tsx +++ b/packages/landing/src/components/checklist/SplitPanelControls.tsx @@ -11,7 +11,6 @@ import { RefreshCwIcon, } from 'lucide-react'; - interface SplitPanelControlsProps { showSecondPanel: boolean; onToggleSecondPanel?: () => void; @@ -23,7 +22,6 @@ interface SplitPanelControlsProps { pdfUrl?: string | null; pdfData?: ArrayBuffer | null; } - export function SplitPanelControls({ showSecondPanel, diff --git a/packages/landing/src/components/checklist/common/LocalTextAdapter.ts b/packages/landing/src/components/checklist/common/LocalTextAdapter.ts index 6e85b3a84..bd84926e1 100644 --- a/packages/landing/src/components/checklist/common/LocalTextAdapter.ts +++ b/packages/landing/src/components/checklist/common/LocalTextAdapter.ts @@ -80,12 +80,12 @@ export function createLocalAdapterFactories( if (sectionKey.startsWith('domain') && questionKey) { path = `${sectionKey}.${questionKey}.comment`; getValue = (cl): string => - ( + (( (cl?.[sectionKey] as Record)?.answers as Record< string, Record > - )?.[questionKey]?.comment as string || ''; + )?.[questionKey]?.comment as string) || ''; getUpdatedState = (prev, newValue) => ({ ...prev, [sectionKey]: { @@ -93,12 +93,10 @@ export function createLocalAdapterFactories( answers: { ...((prev[sectionKey] as Record)?.answers as Record), [questionKey]: { - ...( - ((prev[sectionKey] as Record)?.answers as Record< - string, - Record - >) ?? {} - )[questionKey], + ...(((prev[sectionKey] as Record)?.answers as Record< + string, + Record + >) ?? {})[questionKey], comment: newValue, }, }, @@ -151,12 +149,12 @@ export function createLocalAdapterFactories( if (sectionKey.startsWith('domain') && questionKey) { path = `${sectionKey}.${questionKey}.comment`; getValue = (cl): string => - ( + (( (cl?.[sectionKey] as Record)?.answers as Record< string, Record > - )?.[questionKey]?.comment as string || ''; + )?.[questionKey]?.comment as string) || ''; getUpdatedState = (prev, newValue) => ({ ...prev, [sectionKey]: { @@ -164,12 +162,10 @@ export function createLocalAdapterFactories( answers: { ...((prev[sectionKey] as Record)?.answers as Record), [questionKey]: { - ...( - ((prev[sectionKey] as Record)?.answers as Record< - string, - Record - >) ?? {} - )[questionKey], + ...(((prev[sectionKey] as Record)?.answers as Record< + string, + Record + >) ?? {})[questionKey], comment: newValue, }, }, @@ -178,13 +174,14 @@ export function createLocalAdapterFactories( } else if (sectionKey === 'sectionB' && questionKey) { path = `sectionB.${questionKey}.comment`; getValue = (cl): string => - ((cl?.sectionB as Record>)?.[questionKey]?.comment as string) || ''; + ((cl?.sectionB as Record>)?.[questionKey] + ?.comment as string) || ''; getUpdatedState = (prev, newValue) => ({ ...prev, sectionB: { ...(prev.sectionB as Record), [questionKey]: { - ...((prev.sectionB as Record>)?.[questionKey]), + ...(prev.sectionB as Record>)?.[questionKey], comment: newValue, }, }, diff --git a/packages/landing/src/components/dashboard/LocalAppraisalCard.tsx b/packages/landing/src/components/dashboard/LocalAppraisalCard.tsx index 1fe63a122..6c0016d74 100644 --- a/packages/landing/src/components/dashboard/LocalAppraisalCard.tsx +++ b/packages/landing/src/components/dashboard/LocalAppraisalCard.tsx @@ -9,7 +9,6 @@ import { SimpleEditable } from '@/components/ui/editable'; import { getChecklistMetadata } from '@/checklist-registry'; import { formatRelativeTime } from './utils'; - interface LocalAppraisalCardProps { checklist: { id: string; @@ -24,7 +23,6 @@ interface LocalAppraisalCardProps { onRename?: (name: string) => void; style?: React.CSSProperties; } - export function LocalAppraisalCard({ checklist, diff --git a/packages/landing/src/components/dashboard/ProjectCard.tsx b/packages/landing/src/components/dashboard/ProjectCard.tsx index a412b6fc4..6a8434126 100644 --- a/packages/landing/src/components/dashboard/ProjectCard.tsx +++ b/packages/landing/src/components/dashboard/ProjectCard.tsx @@ -9,14 +9,12 @@ import { useProjectStore, selectProjectStats } from '@/stores/projectStore'; import { type Project } from '@/hooks/useMyProjectsList'; import { formatRelativeTime, getAccentColors } from './utils'; - interface ProjectCardProps { project: Project; onOpen: (id: string) => void; onDelete?: (id: string) => void; style?: React.CSSProperties; } - export function ProjectCard({ project, onOpen, onDelete, style }: ProjectCardProps) { const cachedStats = useProjectStore(state => selectProjectStats(state, project.id)); diff --git a/packages/landing/src/components/dashboard/ProjectsSection.tsx b/packages/landing/src/components/dashboard/ProjectsSection.tsx index 7fe07e5f4..b5d4e2973 100644 --- a/packages/landing/src/components/dashboard/ProjectsSection.tsx +++ b/packages/landing/src/components/dashboard/ProjectsSection.tsx @@ -27,14 +27,12 @@ import { useAnimation } from './useInitialAnimation'; import { ProjectCard } from './ProjectCard'; import { ContactPrompt } from './ContactPrompt'; - interface ProjectsSectionProps { showHeader?: boolean; createModalOpen: boolean; setCreateModalOpen: (open: boolean) => void; onCreateClick?: () => void; } - export function ProjectsSection({ showHeader = true, diff --git a/packages/landing/src/components/dashboard/useInitialAnimation.ts b/packages/landing/src/components/dashboard/useInitialAnimation.ts index b0758844e..a9b0d3a06 100644 --- a/packages/landing/src/components/dashboard/useInitialAnimation.ts +++ b/packages/landing/src/components/dashboard/useInitialAnimation.ts @@ -10,13 +10,11 @@ import { createContext, useContext, useEffect } from 'react'; // Module-level flag -- persists across re-renders, resets on page reload let hasAnimated = false; - interface AnimationHelpers { shouldAnimate: boolean; fadeUp: (delay?: number) => React.CSSProperties; statRise: (delay?: number) => React.CSSProperties; } - export function useInitialAnimation(): AnimationHelpers { const shouldAnimate = !hasAnimated; diff --git a/packages/landing/src/components/dev/DevPanel.tsx b/packages/landing/src/components/dev/DevPanel.tsx index 3fc3a2e7e..33405d653 100644 --- a/packages/landing/src/components/dev/DevPanel.tsx +++ b/packages/landing/src/components/dev/DevPanel.tsx @@ -211,16 +211,20 @@ export function DevPanel() { Connecting... )} - {isProjectContext && (connectionState?.phase === 'connected' || connectionState?.phase === 'synced') && ( - - Connected - - )} - {isProjectContext && connectionState?.phase !== 'connected' && connectionState?.phase !== 'synced' && connectionState?.phase !== 'connecting' && ( - - Disconnected - - )} + {isProjectContext && + (connectionState?.phase === 'connected' || connectionState?.phase === 'synced') && ( + + Connected + + )} + {isProjectContext && + connectionState?.phase !== 'connected' && + connectionState?.phase !== 'synced' && + connectionState?.phase !== 'connecting' && ( + + Disconnected + + )}