Add a temporary solution for handling conflicts between the remote state and client-side cache when moving todo items #37

fulopkovacs · 2026-01-03T10:43:58Z

Summary by CodeRabbit

Bug Fixes
- Insert and update failures now surface as error toasts and trigger data refresh when needed.
Improvements
- Drag/position changes apply immediately for a snappier experience.
- Optimistic updates are kept on successful server sync; failed syncs show an error and refresh data.
Removals
- Server-side batch-update endpoint and debounced batch persistence have been removed.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

…ate and client-side cache when moving todo items

coderabbitai · 2026-01-03T10:44:06Z

Walkthrough

Removes batched/debounced todo-item persistence and the batch API, makes todoItemsCollection generic with a new query key, implements immediate per-item optimistic updates with server PATCH per item, and changes the PATCH API to return a single updated item.

Changes

Cohort / File(s)	Summary
Collection changes `src/collections/todoItems.ts`	Removed `batchUpdateTodoItem`. Added `todoItemsQueryKey` constant. Exported `todoItemsCollection` now created as `createCollection<TodoItemRecord>(...)`. Insert error handling preserved; onUpdate implements optimistic local update, sends server mutation, and returns `refetch: false`.
Component update flow `src/components/TodoBoards.tsx`	Removed paced/debounced batching (`usePacedMutations`, `debounceStrategy`, `batchUpdateTodoItem`) and replaced with immediate per-item persistence via `todoItemsCollection.update(itemId, { boardId, position })`. Drag-end logic now calls direct collection update.
Local API — single-item update `src/local-api/api.todo-items.ts`	`todoItemUpdateData` is now internal (not exported). PATCH handler uses `.returning()` and returns a single updated item (`json(updatedTodoItemData)`); added 404 handling when no item is updated.
Removed batch API route `src/local-api/api.batch.todo-items.ts`, `src/local-api/index.ts`	Deleted batch PATCH route file and removed `/api/batch/todo-items` mapping from `src/local-api/index.ts`, dropping related imports/exports.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Component as TodoBoards
    participant Collection as todoItemsCollection
    participant Server as PATCH /api/todo-items
    participant UI as Toast/Error Handler

    rect rgba(100,150,200,0.12)
    note over Component,Collection: Immediate per-item update
    User->>Component: Drop item (compute boardId, position)
    Component->>Collection: update(itemId, { boardId, position })
    Collection->>Collection: Apply optimistic local change
    Collection-->>Component: Local state updated
    end

    Component->>Server: PATCH item
    alt Server success
        Server-->>Collection: Return updated item
        Collection->>Collection: Confirm/retain optimistic state
    else Server error
        Server-->>Collection: Error
        Collection->>UI: Show error toast
        Collection->>Server: (optional) refetch latest state
        Server-->>Collection: Latest state
        Collection->>Collection: Reconcile / rollback
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Use fractional indexing for the positions of the todo items #20 — Directly touches todo ordering, collection typing, and persistence flows; strongly related.
Adjust the debounce wait #25 — Modifies debounced/batched position updates in TodoBoards; related to removal of batching.
Update the theme #22 — Reworks drag/board/task behaviors in TodoBoards, affecting the same component-level flows.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.
Title check	❓ Inconclusive	The PR title refers to a temporary solution for handling state conflicts, but the actual changes remove batching infrastructure and implement immediate per-item updates with optimistic updates—not explicitly a conflict-resolution mechanism.	Clarify whether the title should emphasize the immediate per-item update approach or specify what type of conflict is being addressed. Consider revising to 'Replace batch todo-item updates with immediate per-item mutations' or similar.

✅ Passed checks (1 passed)

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

✨ Finishing touches

📝 Generate docstrings

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b533889 and a167793.

📒 Files selected for processing (2)

src/collections/todoItems.ts
src/local-api/api.todo-items.ts

🧰 Additional context used

📓 Path-based instructions (3)

src/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Always ensure no formatting/linting issues exist in code - use pnpm check --fix to automatically fix issues

Files:

src/collections/todoItems.ts
src/local-api/api.todo-items.ts

**/*.{tsx,ts}

📄 CodeRabbit inference engine (AGENTS.md)

Prefer functions over arrow functions for React components

Files:

src/collections/todoItems.ts
src/local-api/api.todo-items.ts

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Prefer type over interface for type definitions

Files:

src/collections/todoItems.ts
src/local-api/api.todo-items.ts

🧬 Code graph analysis (2)

src/collections/todoItems.ts (2)

src/db/schema.ts (1)

TodoItemRecord (72-72)

src/local-api/api.batch.todo-items.ts (2)

updatedData (11-49)

tx (39-46)

src/local-api/api.todo-items.ts (2)

src/db/schema.ts (1)

todoItemsTable (54-70)

src/local-api/helpers.ts (1)

json (29-43)

🔇 Additional comments (5)

src/local-api/api.todo-items.ts (2)

18-18: LGTM - appropriate visibility change.

Making todoItemUpdateData internal is consistent with the removal of the batch API and reduces the public API surface.

112-122: LGTM - 404 handling correctly implemented.

The PATCH handler now properly detects when no item is updated and returns a 404 response, addressing the previous review comment. The use of .returning() and single-item response format aligns well with the shift to per-item updates.

src/collections/todoItems.ts (3)

56-60: LGTM - good refactor for maintainability and type safety.

Extracting the query key into a constant and adding the generic type parameter are both excellent improvements that follow best practices.

76-92: LGTM - clear documentation of temporary solution.

The comment block appropriately warns about the temporary nature of this implementation and provides helpful context about proper synchronization approaches.

110-123: Error handling and refetch logic are correct.

The error handling properly:

Shows a user-friendly error toast

Checks isRefetching to prevent duplicate refetch calls

Returns { refetch: false } to disable automatic background refetch

The implementation correctly prevents race conditions while ensuring data consistency on errors.

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

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

coderabbitai

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/collections/todoItems.ts (1)

38-57: Address the unused export warning.

The batchUpdateTodoItem function is exported but never used according to the pipeline failure. Since the PR moves to per-item updates, this appears to be dead code that should either be removed or marked for future use.

Consider either:

Removing the function if it's no longer needed

Removing the export keyword if it's only used internally

Adding a comment explaining why it's kept for future use

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f9aac14 and 6810aaf.

📒 Files selected for processing (3)

src/collections/todoItems.ts
src/components/TodoBoards.tsx
src/local-api/api.todo-items.ts

🧰 Additional context used

📓 Path-based instructions (3)

src/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Always ensure no formatting/linting issues exist in code - use pnpm check --fix to automatically fix issues

Files:

src/components/TodoBoards.tsx
src/local-api/api.todo-items.ts
src/collections/todoItems.ts

**/*.{tsx,ts}

📄 CodeRabbit inference engine (AGENTS.md)

Prefer functions over arrow functions for React components

Files:

src/components/TodoBoards.tsx
src/local-api/api.todo-items.ts
src/collections/todoItems.ts

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Prefer type over interface for type definitions

Files:

src/components/TodoBoards.tsx
src/local-api/api.todo-items.ts
src/collections/todoItems.ts

🧠 Learnings (2)

📚 Learning: 2025-12-24T15:25:07.106Z

Learnt from: fulopkovacs
Repo: fulopkovacs/trytanstackdb.com PR: 22
File: src/components/TodoBoards.tsx:63-81
Timestamp: 2025-12-24T15:25:07.106Z
Learning: In Tailwind CSS v4, prefer canonical classes using bg-linear-to-* (e.g., bg-linear-to-b, bg-linear-to-t, bg-linear-to-r) over the older bg-gradient-to-* syntax. This aligns with Tailwind LSP's suggestCanonicalClasses rule. Apply across TSX files (e.g., src/components/*.tsx); replace occurrences of bg-gradient-to-* with the corresponding bg-linear-to-* equivalents and verify gradients visually.

Applied to files:

src/components/TodoBoards.tsx

📚 Learning: 2025-12-28T23:34:13.416Z

Learnt from: fulopkovacs
Repo: fulopkovacs/trytanstackdb.com PR: 32
File: src/components/HomeIntro.tsx:16-25
Timestamp: 2025-12-28T23:34:13.416Z
Learning: In Motion (motion/react), when configuring variant transitions, use delayChildren: stagger(x) to create per-child delays (e.g., delayChildren: stagger(0.2)). This is different from the older API that used a separate staggerChildren property. Apply this guidance to TSX files that declare motion variants (e.g., in src/components/**/*.tsx) and verify that stagger is imported from framer-motion. Ensure transitions reflect the intended stagger timing and that no conflicting delay values override the staggered effect.

Applied to files:

src/components/TodoBoards.tsx

🧬 Code graph analysis (2)

src/components/TodoBoards.tsx (3)

src/collections/todoItems.ts (1)

todoItemsCollection (79-148)

src/local-api/api.batch.todo-items.ts (1)

updatedData (11-49)

src/components/PriorityRating.tsx (1)

index (33-37)

src/collections/todoItems.ts (1)

src/db/schema.ts (1)

TodoItemRecord (72-72)

🪛 GitHub Actions: CI/CD

src/collections/todoItems.ts

[warning] 38-38: Unused exports: 'batchUpdateTodoItem' is defined but never used. (no-unused-vars)

🔇 Additional comments (5)

src/collections/todoItems.ts (3)

55-56: Type assertion is unavoidable here.

The TODO comment about avoiding type casting is noted. Since the API contract is established and validated server-side with Zod schemas, this type assertion is reasonable and safe.

77-81: Good practice: Extracting the query key constant.

Defining todoItemsQueryKey as a constant and using it in the collection options improves maintainability and makes it easier to reference the same query key elsewhere if needed.

97-144: Remove the manual writeUpdate call — it's redundant.

The optimistic update is already applied when collection.update() is called (before the onUpdate handler runs). Calling writeUpdate() in the success path with identical changes has no effect and adds unnecessary code. Since you're returning { refetch: false }, the optimistic state already stands after server confirmation. If you need to fetch the latest server state instead, remove refetch: false and let TanStack handle the sync automatically.

src/components/TodoBoards.tsx (2)

307-322: Clean refactoring from batch to per-item updates.

The new updateTodoPosition function simplifies the update logic by directly mutating the item through the collection's update method. The implementation correctly handles both position-only updates and board changes, with error handling delegated to the collection's onUpdate handler.

400-404: LGTM: Consistent usage of the new update pattern.

All three call sites correctly use updateTodoPosition with appropriate parameters for their specific scenarios (empty column drop, same-column reorder, and cross-column move).

Also applies to: 427-430, 449-453

src/local-api/api.todo-items.ts

github-actions · 2026-01-03T10:48:23Z

🚀 Preview Deployment

Status: ✅ Ready!

Preview URL: Open Preview

Commit: a16779385bab4ea965e7ba891ff1dce67de1bb04

Built and deployed successfully

coderabbitai

Actionable comments posted: 2

♻️ Duplicate comments (1)

src/local-api/api.todo-items.ts (1)
112-118: Add error handling for when the todo item is not found.

The destructuring assumes the update will always find a matching item. If updatedData.id doesn't exist in the database, updatedTodoItemData will be undefined, causing the endpoint to return json(undefined) with a 200 status code instead of a proper error response.
🔎 Proposed fix
 const [updatedTodoItemData] = await db
   .update(todoItemsTable)
   .set(updatedData)
   .where(eq(todoItemsTable.id, updatedData.id))
   .returning();

+if (!updatedTodoItemData) {
+  return new Response(`Todo item not found`, { status: 404 });
+}
+
 return json(updatedTodoItemData);

🧹 Nitpick comments (1)

src/collections/todoItems.ts (1)
110-110: Log the error for better debugging.

The error is caught with _ but not logged, making it harder to debug API failures. This is inconsistent with the error handling in the onInsert callback (line 72) which logs errors to the console.
🔎 Proposed fix
-     } catch (_) {
+     } catch (error) {
        toast.error(`Failed to update todo item "${original.title}"`);
+       console.error("Failed to update todo item:", error);

        // Abort all ongoing sync queries

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6810aaf and b533889.

📒 Files selected for processing (4)

src/collections/todoItems.ts
src/local-api/api.batch.todo-items.ts
src/local-api/api.todo-items.ts
src/local-api/index.ts

💤 Files with no reviewable changes (2)

src/local-api/api.batch.todo-items.ts
src/local-api/index.ts

🧰 Additional context used

📓 Path-based instructions (3)

src/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Always ensure no formatting/linting issues exist in code - use pnpm check --fix to automatically fix issues

Files:

src/local-api/api.todo-items.ts
src/collections/todoItems.ts

**/*.{tsx,ts}

📄 CodeRabbit inference engine (AGENTS.md)

Prefer functions over arrow functions for React components

Files:

src/local-api/api.todo-items.ts
src/collections/todoItems.ts

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Prefer type over interface for type definitions

Files:

src/local-api/api.todo-items.ts
src/collections/todoItems.ts

🧬 Code graph analysis (2)

src/local-api/api.todo-items.ts (3)

src/db/index.ts (1)

db (13-15)

src/db/schema.ts (1)

todoItemsTable (54-70)

src/local-api/helpers.ts (1)

json (29-43)

src/collections/todoItems.ts (1)

src/db/schema.ts (1)

TodoItemRecord (72-72)

🔇 Additional comments (2)

src/collections/todoItems.ts (2)

56-60: LGTM! Good refactor to improve type safety and maintainability.

The introduction of todoItemsQueryKey as a constant and making the collection generic with TodoItemRecord improves code maintainability and type inference.

76-92: Good documentation of the temporary nature of this solution.

The warning about not using this in production and the clear explanation of the update strategy helps reviewers understand the limitations and trade-offs.

src/collections/todoItems.ts

Add a temporary solution for handling conflicts between the remote st…

6810aaf

…ate and client-side cache when moving todo items

coderabbitai bot reviewed Jan 3, 2026

View reviewed changes

src/local-api/api.todo-items.ts Show resolved Hide resolved

Remove the unused todo items batch update endpoint

b533889

fulopkovacs deployed to Preview January 3, 2026 10:48 — with GitHub Actions View deployment

coderabbitai bot reviewed Jan 3, 2026

View reviewed changes

src/collections/todoItems.ts Show resolved Hide resolved

src/collections/todoItems.ts Show resolved Hide resolved

fulopkovacs added 2 commits January 3, 2026 11:55

Make the API throw the error if the todo item is not found

de2f1e9

Fix a comment

a167793

fulopkovacs deployed to Preview January 3, 2026 11:03 — with GitHub Actions View deployment

fulopkovacs merged commit d9d8cbe into main Jan 3, 2026
4 checks passed

fulopkovacs deleted the fix-moving-todo-items branch January 3, 2026 11:16

coderabbitai bot mentioned this pull request Jan 3, 2026

Use on demand sync strategy for the todo items #38

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a temporary solution for handling conflicts between the remote state and client-side cache when moving todo items #37

Add a temporary solution for handling conflicts between the remote state and client-side cache when moving todo items #37

Uh oh!

fulopkovacs commented Jan 3, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Jan 3, 2026 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

github-actions bot commented Jan 3, 2026 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Add a temporary solution for handling conflicts between the remote state and client-side cache when moving todo items #37

Add a temporary solution for handling conflicts between the remote state and client-side cache when moving todo items #37

Uh oh!

Conversation

fulopkovacs commented Jan 3, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Jan 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Jan 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🚀 Preview Deployment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fulopkovacs commented Jan 3, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Jan 3, 2026 •

edited

Loading

github-actions bot commented Jan 3, 2026 •

edited

Loading