Sync: multi-select pull/push in the sync modal

[MrLinks75]

Adds checkboxes to the Pull/Push story lists so multiple stories can be
synced in one go instead of one at a time. Each story is still pulled or
pushed via its own authenticated request, conflict check, and pre-sync
backup, run sequentially, the batch is just a loop over the existing
per-story flow.

Summary by CodeRabbit

• New Features
  • Added support for selecting and transferring multiple stories at once.
  • Added select-all controls for streamlined batch selection.
  • Added independent pull and push actions for greater control.
  • Added conflict previews before pulling stories.
  • Added per-story progress tracking with success and failure reporting.
  • Added clearer batch transfer status for both pull and push operations.
  • Improved story replacement handling during transfers.

[coderabbitai]

Warning

Review limit reached

@MrLinks75, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 18 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 360060b4-195d-43d0-b4bb-b4c231fe43df

📥 Commits

Reviewing files that changed from the base of the PR and between 6543b84 and 29ef984.

📒 Files selected for processing (1)

• src/lib/components/sync/SyncModal.svelte

📝 Walkthrough

Walkthrough

SyncModal now supports selecting multiple remote or local stories. It processes pulls and pushes sequentially, checks pull conflicts, imports before replacement deletion, tracks progress, and reports aggregate results.

Changes

Multi-story sync

Layer / File(s)	Summary
Selection state and controls src/lib/components/sync/SyncModal.svelte	The modal stores remote and local story selections, resets batch state, and provides select-all and multi-selection controls. The connected-mode text and footer actions show selected story counts.
Sequential batch transfers src/lib/components/sync/SyncModal.svelte	Pulls check all conflicts before transfer and import replacements before deleting existing copies. Pulls and pushes run sequentially with per-story progress, failure collection, story reloads, and aggregate results. Conflict and syncing displays show batch details.

Estimated code review effort: 4 (Complex) | ~45 minutes

Suggested reviewers: a-frazier

Poem

I’m a rabbit with stories in a row,
Select them all and watch them go.
Pull or push through conflicts bright,
Track each story’s progress right.
Batch sync hops into the night.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: multi-selection for pull and push operations in the sync modal.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

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.}

[Copilot]

Pull request overview

Adds multi-select support to the Sync modal so users can pull/push multiple stories in one operation, while keeping the existing per-story sync flow (auth request, conflict check, pre-sync backup) and running it sequentially per selected story.

Changes:

• Replaced single-story selection with multi-select (checkbox-style UI) for both remote (pull) and local (push) story lists.
• Implemented sequential batch pull/push loops with progress display and summarized success/failure messaging.
• Expanded conflict warning UX to handle multiple conflicting story titles.

Suppressed comments (1)

src/lib/components/sync/SyncModal.svelte:467

• ui.setSyncMode('syncing') happens before syncDirection = 'push'. If the syncing state renders between these updates, the UI may briefly show “Pushing…” with an incorrect direction (or default). Set syncDirection first.

    ui.setSyncMode('syncing')
    syncDirection = 'push'
    loading = true

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[coderabbitai]

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/components/sync/SyncModal.svelte`:
- Around line 474-478: Preserve the currently active story in the batch-push
flow around the storiesToPush loop: capture the active story ID before
iterating, then restore that story after all pushes complete because
createPreSyncBackup can update StoryStore.currentStory. Keep each story’s backup
and push behavior unchanged, and ensure restoration also occurs when the batch
finishes through the existing completion path.
- Around line 405-417: Bind conflict confirmation to the selected remote-story
snapshot: in src/lib/components/sync/SyncModal.svelte lines 405-417, store the
checked story IDs and process only that snapshot after confirmation instead of
skipping checks based solely on showConflictWarning. In lines 719-739, prevent
selection changes while confirmation is pending, or clear the pending snapshot
and warning whenever the selection changes.
- Around line 454-456: Update the sync flow around story.loadAllStories() so
loading is always reset to false in a finally block, including when the refresh
rejects. Ensure reportBatchResult('pulled', ...) still executes appropriately
after a successful refresh without being skipped by the new error-handling
structure.
- Around line 433-441: Update the sync flow around findStoryIdByTitle,
pullStory, and importFromContent so it fetches the remote story and retains a
durable local export before deleting the existing story. Only call deleteStory
after the replacement import succeeds, preserving the pre-sync data until import
completion; do not rely on restoreCheckpoint.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d93d6cc7-9d0e-4887-b624-69169b684ed1

📥 Commits

Reviewing files that changed from the base of the PR and between 6d18d69 and 48fb9e0.

📒 Files selected for processing (1)

• src/lib/components/sync/SyncModal.svelte

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/components/sync/SyncModal.svelte`:
- Around line 722-726: Update the conflict messaging in SyncModal’s
conflictStoryTitles branch to replace “downloaded” with “imported,” accurately
reflecting that existing copies are removed only after importFromContent()
completes successfully.
- Around line 145-154: Make replacement imports atomic in the received-story
flow around importFromContent and the pulled-story flow at
src/lib/components/sync/SyncModal.svelte lines 442-459: use a transactional
replacement operation, or if unavailable, remove the newly imported copy when
deleteStory fails. Ensure failures do not leave both copies or report failure
after local data has already been inconsistently changed.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 202cc844-6941-45b6-8b79-8f950b5e6902

📥 Commits

Reviewing files that changed from the base of the PR and between 48fb9e0 and 6543b84.

📒 Files selected for processing (1)

• src/lib/components/sync/SyncModal.svelte

[a-frazier]

Code review

Found 1 issue:

1. Batch-pulling two remote stories that share the same title silently destroys the first one pulled. Each loop iteration calls findStoryIdByTitle(s.title) (which returns the most-recently-updated story with that title, per getAllStories()'s ORDER BY updated_at DESC), so on the second iteration the lookup returns the story just imported by the first iteration, and the delete-the-old-copy step removes it — while succeeded already recorded it as pulled. The upfront conflict check only compares selected titles against existing local stories, not against duplicate titles within the selection itself, so nothing warns the user. This couldn't happen before the PR since only one story could be pulled at a time.

Aventuras/src/lib/components/sync/SyncModal.svelte

Lines 446 to 468 in 29ef984

	// Look up the existing same-titled story before importing: the freshly imported
	// story shares that title and sorts first by updated_at, so finding it afterward
	// would return the new story's own id instead of the one to replace.
	const existingId = await syncService.findStoryIdByTitle(s.title)
	// Import the replacement before deleting anything, so a failed pull/import
	// never leaves the device with neither copy.
	const storyJson = await syncService.pullStory(conn, s.id)
	// Use skipImportedSuffix=true so synced stories keep their original title
	const result = await exportService.importFromContent(storyJson, true)
	if (!result.success) {
	failed.push(s.title)
	} else if (existingId) {
	try {
	await syncService.deleteStory(existingId)
	succeeded.push(s.title)
	} catch {
	// The new copy imported fine but removing the old one failed: roll back
	// the new copy rather than leave two stories with the same title.
	await syncService.deleteStory(result.storyId!).catch(() => {})
	failed.push(s.title)
	}

A pre-scan of storiesToPull for duplicate titles (surfacing them in the conflict warning), or resolving all existingIds before the loop starts, would close the gap.

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[Pento95]

Thanks for this — import-before-delete genuinely fixes a case where a failed download left the device with neither copy, and restoring the previously open story after a push batch is a good catch. +1 to @a-frazier's duplicate-title finding; a few more things I'd like you to check, in case I've misread the flow.

The pre-sync backup is gone from both pull paths. master called createPreSyncBackup(existingId) before deleteStory(existingId) in pullStory and importReceivedStory; only the push one survives. The new ordering protects against a failed import, but the backup protected against something else — pulling an older copy over newer local work — and that's now unrecoverable. The conflict warning was reworded away from it too, so it isn't visible from the UI. Intended?

A throw outside the per-item try leaves the modal stuck. pullSelectedStories has try/finally but no catch (master wrapped the whole body). If await story.loadAllStories() throws, reportBatchResult never runs and setSyncMode('connected') is skipped — and the template renders the spinner for syncMode === 'syncing' regardless of loading, so it's a permanent spinner with no footer and no Done. Same shape on push, where story.closeStory() sits unguarded before loading = false.

A partial failure renders as a full success. reportBatchResult sets syncSuccess = true as soon as one story succeeds, and the success block is a hardcoded green check with "Sync Complete!" — 1-of-5 and 5-of-5 look identical, with the failures only in the body text.

Double-clicking "Pull N Selected" can start two batches. The conflict pre-check runs N sequential await checkStoryExists calls before loading = true, and the button is only disabled on !loading — so it stays live for the whole pre-check, and that window grows with the selection size. Setting loading = true first would close it.

Heads-up: this and #446 both rewrite pullStory and importReceivedStory in the same file, so whichever lands second needs a real merge.