Skip to content

Replay pre-mount file events so resumed watches show existing files - #25

Merged
cohogan merged 4 commits into
mainfrom
f/replay-file-change-backlog
Aug 7, 2026
Merged

Replay pre-mount file events so resumed watches show existing files#25
cohogan merged 4 commits into
mainfrom
f/replay-file-change-backlog

Conversation

@cohogan

@cohogan cohogan commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Problem

When the app restarts and resumes watching, the initial folder scan runs during setup() — before the webview has mounted and registered its event listeners. Tauri drops emits that have no listener, and the frontend's file list is built purely from live file_change events, so after a restart the UI showed no files even though the scan ran and uploads proceeded. Live watch events still worked, which made it look like only the "old" files were missing. Per-file upload status icons were lost the same way. (The "ignore existing files" setting was unrelated — it only gates upload queueing.)

Fix

The backend now records what it emits, and the frontend replays the backlog on mount:

  • FileChangeLog managed state: latest event per path, capped at 500 (matching the frontend list cap). Recorded by record_and_emit_file_change at both emit sites (watcher callback and initial scan), exposed via get_file_change_backlog, cleared on stop_watching and when a new watch starts — mirroring when the frontend clears its list.
  • UploadStatusLog managed state: latest status per relative path, capped at 1000. Recorded inside emit_file_upload_status, exposed via get_upload_status_backlog.
  • Recording happens before emitting, and the frontend registers listeners before pulling the backlogs, so a frontend mounting mid-scan sees every event either live or in the snapshot — never neither. Live entries win merge conflicts.

Bumps version to 0.1.17 (0.1.16 is taken by #24).

Verification

  • cargo check and cargo clippy clean, tsc --noEmit clean
  • Not verified by launching the app: dev shares the installed app's store/identifier and would trigger real uploads. Manual test: watch a folder, quit, reopen — existing files should populate the list with status icons.

Note

Conflicts with #24, which rewrites the same functions (capture_initial_contentsscan_folder_contents, runtime-generic signatures). Suggested order: merge #24 first; this branch then needs a rebase adapting the helpers to be generic over tauri::Runtime.

Co-Authored-By: Claude Fable 5 noreply@anthropic.com

cohogan and others added 2 commits August 6, 2026 20:38
When watching resumes at startup, the initial scan runs before the
webview has registered its event listeners, and Tauri drops emits that
have no listener. The frontend's file list is built purely from live
file_change events, so every existing file was silently missing after a
restart; live watch events still worked, which made it look like the
scan never ran. Per-file upload status icons were lost the same way.

The backend now records what it emits: a FileChangeLog (latest event
per path, capped at 500 to match the frontend list) and an
UploadStatusLog (latest status per relative path, capped at 1000).
Recording happens before emitting, so a frontend mounting concurrently
sees each event either live or in the backlog, never neither. On mount
the frontend registers its listeners first, then pulls
get_file_change_backlog / get_upload_status_backlog and merges, with
live entries winning conflicts. The change log is cleared on
stop_watching and when a new watch starts, mirroring the frontend
clearing its list at those points.

Bumps version to 0.1.17.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cursor

cursor Bot commented Aug 7, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

Claude finished @cohogan's task in 2m 8s —— View job


Code Review: PR #25 — Replay pre-mount file events

  • Read PR diff and changed files
  • Review Rust backend changes (lib.rs, upload.rs)
  • Review frontend changes (simple.tsx)
  • Review version bump and other changes
  • Post review findings

Overall this is a well-structured fix for a real problem — the race between setup() scanning and webview mount. The "record before emit, listen before pull" protocol is correct and the PR description explains the invariant clearly. A few issues worth addressing:


Bug: UploadStatusLog is never cleared on stop/start

FileChangeLog is cleared in both stop_watching (lib.rs:315) and start_watching_impl (lib.rs:160), mirroring the frontend which calls setFileChanges([]) on both paths. But UploadStatusLog is never cleared — not on stop, not on start of a new folder.

This means after Stop → Watch a different folder, get_upload_status_backlog will replay stale upload statuses from the previous folder. The frontend also doesn't clear uploadStatuses on stop/start (simple.tsx:317 clears fileChanges but not uploadStatuses), so this was probably a latent bug before this PR, but the backlog replay makes it more visible since it now actively injects the stale data on mount.

The frontend should clear uploadStatuses alongside fileChanges in selectAndWatchFolder and stopWatching, and the backend should clear UploadStatusLog in the same places it clears FileChangeLog.

Fix this →


Performance: Vec::remove(0) is O(n) in UploadStatusLog eviction

In upload.rs:218:

if log.len() >= MAX_UPLOAD_STATUS_LOG {
    log.remove(0);
}

Vec::remove(0) shifts all elements left — O(n) with n up to 1,000. The FileChangeLog correctly uses VecDeque::pop_front() for O(1) eviction. UploadStatusLog should use VecDeque as well for consistency and to avoid the linear shift on eviction.

That said, with a 1,000 cap and small structs, this is unlikely to be a real bottleneck — more of a consistency concern.

Fix this →


Performance: O(n) linear search per emit in UploadStatusLog

In upload.rs:214:

if let Some(existing) = log.iter_mut().find(|s| s.relative_path == relative_path) {
    *existing = upload_status.clone();
}

This is O(n) per call. emit_file_upload_status is called for every file during the initial scan and every upload lifecycle event. For a folder with thousands of files, this means thousands of O(n) scans = O(n²) total.

Similarly, FileChangeLog uses VecDeque::retain in record_and_emit_file_change (lib.rs:99), which is also O(n) per event — same quadratic concern for large scans.

Both could use a HashMap<String, ...> alongside the ordered collection for O(1) dedup lookups. But given the caps (500/1000), this is likely fine for real-world folder sizes. Worth noting if users report sluggishness on initial scan of very large folders.


Minor: Frontend backlog merge appends instead of prepends

In simple.tsx:213:

const merged = [
  ...prev,
  ...backlog.filter((change) => !seen.has(change.path)),
];

The backlog is returned newest-first from get_file_change_backlog (lib.rs:339: iter().rev()). Live events in prev are also newest-first (the file_change listener prepends at simple.tsx:177). So backlog entries appear after live events in the merged array. This means if the UI sorts by array position, files that arrived via backlog will appear below live-received files even if they are newer.

In practice this is probably correct — live events are newer than the backlog since the backlog is a snapshot from before mount. But if the scan is still running when mount happens, some backlog entries could be newer than early live events. The seen set prevents duplicates, so it's not broken — just a subtle ordering quirk to be aware of.


Nit: CLAUDE.md change is unrelated

The CI/CD version-bump instruction added to CLAUDE.md is orthogonal to this PR's fix. Consider splitting it into its own commit or PR for cleaner history, or noting it in the PR description.


Summary

The core design is sound — the record-before-emit / listen-before-pull protocol correctly closes the race window. The main actionable item is clearing UploadStatusLog on stop/start to match the FileChangeLog lifecycle and prevent stale status replay across folder changes. The Vec::remove(0)VecDeque::pop_front() change is a clean improvement. The O(n²) concern is theoretical at current caps but worth keeping in mind.

cohogan and others added 2 commits August 6, 2026 20:46
…backlog

# Conflicts:
#	package.json
#	src-tauri/Cargo.lock
#	src-tauri/Cargo.toml
#	src-tauri/src/lib.rs
#	src-tauri/src/upload.rs
#	src-tauri/tauri.conf.json
Review follow-ups on the backlog replay:

The frontend fetched the backlogs before the file_upload_status listener
was registered (and listen() registration is itself async), leaving the
same drop-window the backlog exists to close. Both fetches now wait on
the listener registration promises, so every event is seen either live
or in the snapshot.

New event_backlog integration tests cover the recording paths with
MockRuntime: scan records every entry, one entry per path across
rescans, cap-and-evict for both logs, latest-status-wins with stable
position, start_watching clearing the previous folder's backlog, and
graceful no-op when the logs are not managed. Exports the touched
types doc-hidden, following the watch_supersede pattern.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

Claude finished @cohogan's task in 2m 12s —— View job


Code Review: PR #25 — Replay pre-mount file events

  • Read PR diff and all changed files
  • Review Rust backend changes (lib.rs, upload.rs)
  • Review frontend changes (simple.tsx)
  • Review test file (event_backlog.rs)
  • Review version bump and other changes
  • Post review findings

The core design is solid — "record before emit, listen before pull" correctly closes the race between setup() scanning and webview mount. Test coverage is thorough and the integration tests exercise all the important invariants (dedup, cap, eviction, clearing, graceful degradation). A few things to address:


Bug: UploadStatusLog is never cleared on stop/start

FileChangeLog is correctly cleared in both stop_watching (lib.rs:403) and start_watching_impl (lib.rs:185). But UploadStatusLog is never cleared anywhere — not on stop, not on a new watch start.

After Stop → Watch a different folder, get_upload_status_backlog will replay stale upload statuses from the previous folder. The frontend also doesn't clear uploadStatuses on stop/start — selectAndWatchFolder (simple.tsx:322) resets fileChanges but not uploadStatuses, and stopWatching (simple.tsx:347) does the same.

This was harmless before this PR (the status map just accumulated dead entries), but the backlog replay now actively injects stale data from the old folder on mount.

Fix: Clear UploadStatusLog in both stop_watching and start_watching_impl alongside FileChangeLog, and add setUploadStatuses(new Map()) alongside setFileChanges([]) in both selectAndWatchFolder and stopWatching.

Fix this →


Performance: Vec::remove(0) is O(n) in UploadStatusLog eviction

In upload.rs:219:

if log.len() >= MAX_UPLOAD_STATUS_LOG {
    log.remove(0);
}

Vec::remove(0) shifts all elements — O(n) with n up to 1,000. FileChangeLog correctly uses VecDeque::pop_front() for O(1) eviction. UploadStatusLog should use VecDeque too for consistency.

At the current 1,000 cap this won't be a real bottleneck, but it's a trivial fix and avoids an inconsistency between the two logs.

Fix this →


Nit: record_and_emit_file_change uses retain for dedup — O(n) per call

lib.rs:114:

log.retain(|e| e.path != event.path);

This scans the entire deque on every file change event. During the initial scan of a folder with thousands of files this is O(n²) total (though n is capped at 500). The same concern applies to UploadStatusLog's iter_mut().find() at upload.rs:215.

Both could use a HashSet<String> alongside the ordered collection for O(1) membership checks. Given the 500/1000 caps, this is likely fine for practical folder sizes — flagging it mainly as context if users report sluggishness scanning large folders.


Good: Frontend listener-before-backlog protocol

The Promise.all([unlistenFileChange, unlistenUploadStatus]).then(...) pattern at simple.tsx:225 correctly waits for Tauri listener registration to complete before pulling backlogs. This ensures every event is seen either live or in the snapshot. The .catch(() => undefined) fallback is a reasonable defensive choice — listener registration rarely fails in Tauri, and partial coverage is better than none.


Good: try_state for graceful degradation

Using try_state::<FileChangeLog>() / try_state::<UploadStatusLog>() instead of state() means the logging is a no-op when the managed state hasn't been registered (e.g., in existing tests that don't register the logs). The recording_is_skipped_gracefully_when_logs_are_not_managed test validates this explicitly. Nice touch.


Nit: CLAUDE.md change is unrelated

The version-bump-on-every-PR instruction added to CLAUDE.md is orthogonal to this fix. Consider splitting it into its own commit for a cleaner history, or noting it in the PR description.


Summary

The architecture is well-thought-out and the test coverage is thorough. The one actionable bug is clearing UploadStatusLog (and frontend uploadStatuses) on stop/start to prevent stale status replay across folder changes. The Vec::remove(0)VecDeque::pop_front() change is a clean consistency improvement. Everything else looks good to merge.
|

@cohogan
cohogan merged commit 4c3bffc into main Aug 7, 2026
1 check passed
@cohogan
cohogan deleted the f/replay-file-change-backlog branch August 7, 2026 04:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant