Refactor sync 2

[InfinityBowman]

Summary by CodeRabbit

• Refactor

  • Improved internal architecture for managing project operations with better organization and type safety across key domains.
  • Enhanced data synchronization efficiency with incremental update tracking and lifecycle controls, reducing unnecessary re-renders and improving responsiveness.
  • Consolidated and streamlined internal action modules for better maintainability.

• Tests

  • Updated E2E test assertions with improved formatting and visibility checks.

[coderabbitai]

Warning

Rate limit exceeded

@InfinityBowman has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 6 minutes and 20 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 6 minutes and 20 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bcf93633-787b-4386-ae9d-646841a95993

📥 Commits

Reviewing files that changed from the base of the PR and between a6eb97e and a62df31.

⛔ Files ignored due to path filters (1)

• packages/docs/audits/yjs-sync-pipeline-redesign-b.md is excluded by !packages/docs/audits/**

📒 Files selected for processing (5)

• packages/landing/src/project/actions.ts
• packages/landing/src/project/actions/members.ts
• packages/landing/src/project/actions/pdfs.ts
• packages/landing/src/project/actions/project.ts
• packages/landing/src/project/actions/studies.ts

📝 Walkthrough

Walkthrough

Refactors connection/sync architecture: introduces typed per-domain project ops via ConnectionPool.getOps, replaces flat ops registry, overhauls SyncManager with attach/detach/pause/resume and incremental sync, inlines project action modules to call namespaced ops, and updates many consumers and formatting across the codebase.

Changes

Cohort / File(s)	Summary
Core connection & typed ops packages/landing/src/project/ConnectionPool.ts, packages/landing/src/project/connectionReducer.ts, packages/landing/src/primitives/useProject/connection.ts	Replaced flat ConnectionOps with TypedProjectOps (namespaced per-domain ops). Updated connection entry typing, event handler wiring, replaced legacy phase mapper, and switched to syncManager.attach/detach semantics.
Sync manager overhaul packages/landing/src/primitives/useProject/sync.ts	Reworked SyncManager: removed syncFromYDoc, added lifecycle methods (attach, detach, pause, resume), introduced incremental sync via dirtySlices, studyCache, observers, scheduleSync, and helper rebuild functions.
Project actions consolidation packages/landing/src/project/actions.ts, packages/landing/src/project/actions/checklists.ts (removed), packages/landing/src/project/actions/outcomes.ts (removed), packages/landing/src/project/actions/reconciliation.ts (removed)	Inlined previously separate action modules into actions.ts; new project singleton methods call namespaced ops (ops.study, ops.checklist, ops.pdf, ops.reconciliation, ops.outcome) and add error/toast handling and user-id checks. Removed old action modules.
Study & PDF ops updates packages/landing/src/project/actions/studies.ts, packages/landing/src/project/actions/pdfs.ts, packages/landing/src/project/actions/project.ts	Updated callers to use TypedProjectOps and namespaced ops (ops.study.*, ops.pdf.*), normalized metadata nullability, added PDF type narrowing, and migrated some REST calls to RPC client usage.
Checklist & reconciliation consumers packages/landing/src/components/checklist/ChecklistYjsWrapper.tsx, packages/landing/src/components/checklist/common/LocalTextAdapter.ts, packages/landing/src/components/project/reconcile-tab/ReconciliationWrapper.tsx, packages/landing/src/components/project/reconcile-tab/*/navbar-utils.ts	Switched consumers to connectionPool.getOps(projectId) and to ops.checklist/ops.reconciliation access. Hardened checklist answer spreads (cast to Record<string, unknown>), adjusted text accessors and type casts.
Completed/previous-review views packages/landing/src/components/project/completed-tab/CompletedTab.tsx, packages/landing/src/components/project/completed-tab/PreviousReviewersView.tsx	Switched to getOps(...), adapted calls to ops.reconciliation.getAllReconciliationProgress and ops.checklist.*, and made checklist answer spreading defensive.
Annotations & primitives types packages/landing/src/primitives/useProject/annotations.ts, packages/landing/src/primitives/useProject/reconciliation.ts, packages/landing/src/primitives/useProject/pdfs.ts	Adjusted operation interfaces/signatures (explicit checklistId in annotation APIs, formatting/type refinements) to match typed ops surface.
Connection pool consumers & UI multiple components packages/landing/src/components/**, packages/landing/src/project/**, packages/landing/src/stores/**	Updated many components and hooks to consume the new getOps API; mostly formatting and whitespace cleanup, plus minor call-site adaptations to namespaced ops.
Store sync behavior packages/landing/src/stores/projectStore.ts	Changed setProjectData to always assign provided meta/members/studies (removed structural equality guards), and recompute stats unconditionally when studies provided.
Misc: formatting, tests, workers, libs packages/landing/e2e/*.spec.ts, packages/workers/src/**/*, packages/landing/src/lib/*, packages/landing/src/hooks/*	Widespread non-functional formatting: blank-line removal, reflowed multi-line signatures/expressions, minor defensive casts, and test spec assertion reformatting. No behavior changes except where noted above.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant App as App / UI
  participant ConnPool as ConnectionPool (getOps)
  participant Sync as SyncManager
  participant YDoc as Y.Doc
  participant Ops as TypedProjectOps (study, checklist, pdf, reconciliation, annotation)

  rect rgba(93, 173, 226, 0.5)
  App->>ConnPool: request ops for projectId
  ConnPool-->>Sync: ensure syncManager attached
  ConnPool->>Ops: return TypedProjectOps
  end

  rect rgba(144, 238, 144, 0.5)
  note right of Sync: lifecycle attach/observe
  Sync->>YDoc: attach(ydoc)
  YDoc-->>Sync: observeDeep events (reviews/members/meta)
  Sync->>Sync: mark dirtySlices, scheduleSync()
  Sync->>ConnPool: update studyCache / sortedStudies
  end

  rect rgba(249, 206, 106, 0.5)
  App->>Ops: call ops.checklist.updateAnswer(...)
  Ops-->>Sync: enqueue local update -> Yjs update
  Sync->>YDoc: apply update via Yjs
  YDoc-->>Ops: propagate remote changes (via observers)
  Ops-->>App: UI updates via store/hooks
  end

Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~110 minutes

Possibly related issues

• Refactor: Deepen project sync module (Design C + borrowed elements) #424: Implements the RFC-like sync/connection refactor (typed ops, SyncManager lifecycle, getOps) described by the issue.

Possibly related PRs

• 150 improve yjs handling #151 — Both modify Yjs synchronization and SyncManager/connection lifecycle; likely overlap in observer/attach logic.
• 104 refactor project state handlers #105 — Both refactor how project actions and ops are exposed/consumed (typed/namespaced ops and centralized project handlers).
• 202 pdf annotations #323 — Both touch annotation operation signatures and wiring into the per-project ops surface; likely closely related changes.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 37.66% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'Refactor sync 2' is vague and generic, using non-descriptive terminology that doesn't convey meaningful information about the changeset's primary purpose.	Replace with a more descriptive title that captures the main refactoring objective, such as 'Refactor SyncManager with attach/detach lifecycle and connection pool API overhaul' or a similar specific summary.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor-sync-2

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Updated (UTC)
✅ Deployment successful! View logs	corates	a62df31	Mar 29 2026, 03:10 AM

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Updated (UTC)
🔵 In progress View logs	corates	4925665	Mar 29 2026, 02:48 AM

[InfinityBowman]

Code Review

Reviewed the full 97-file, +1592/-1184 diff. The refactor is well-structured -- the new ConnectionPool.getOps() typed namespaced API is a big improvement over the flat untyped ops map, and the incremental observation pipeline (collections with observeDeep) is solid. Observer cleanup is correct, pause()/resume() around Dexie state loading is properly guarded, and the old buildOpsMap/phaseToLegacy/connectionPool.get() are fully removed with no remaining references.

Issues Found

1. Dead method applyReconciliationToChecklists cast through any (Critical)

actions.ts:145:

return (ops.reconciliation as any).applyReconciliationToChecklists(
  studyId, checklist1Id, checklist2Id, data,
);

ReconciliationOperations does not define this method. createReconciliationOperations only exposes saveReconciliationProgress, getReconciliationProgress, getAllReconciliationProgress, and clearReconciliationProgress. This call silently returns undefined at runtime. The as any cast hides the error. Either implement it or remove project.reconciliation.applyToChecklists and its callers.

2. source: 'google-drive' field dropped from Google Drive PDF metadata (Important)

The typed migration in actions/pdfs.ts removed source: 'google-drive' from the metadata passed to addPdfToStudy because PdfInfo doesn't define a source field. All new Google Drive PDFs will permanently lack this source annotation. Worth checking if anything reads pdf.source === 'google-drive' downstream.

3. project.checklist.updateAnswer now writes note to Y.Doc (behavior change)

The old flat ops map silently dropped the 5th note argument because the real function signature only takes 4 params. The new code correctly wraps it into { answer, note }. This is technically a fix, but it's a silent behavior change -- callers that passed note will now have it persisted for the first time. Low risk since the note field is intentional, but worth being aware of.

Confirmed Clean

• Observer cleanup in detach(): all three Y.Map observers properly unregistered via _cleanupHandlers
• pause()/resume() around persisted state loading: correctly prevents observers firing during Dexie hydration
• phaseToLegacy deletion: no remaining consumers
• connectionPool.get() (old untyped): fully replaced, zero remaining call sites
• buildOpsMap deletion: complete, no references remain
• Memory: cleanupHandlers array cleared on destroyEntry, no listener leaks

Looks good overall. The applyReconciliationToChecklists issue is the only one that needs action before merge.