Skip to content

Add autosave, crash recovery, and project history - #1602

Merged
giswqs merged 5 commits into
mainfrom
fix/issue-1522-project-history
Aug 1, 2026
Merged

Add autosave, crash recovery, and project history#1602
giswqs merged 5 commits into
mainfrom
fix/issue-1522-project-history

Conversation

@giswqs

@giswqs giswqs commented Aug 1, 2026

Copy link
Copy Markdown
Member

Summary

  • autosave canonical project snapshots to a bounded IndexedDB ring buffer after a 3-second debounce
  • offer recovery when a newer autosave follows an unclean session
  • add Project > History for browsing and restoring timestamped snapshots
  • clear recovery history when starting a blank new project

Snapshots are capped at 10 MB each, 20 entries, and 50 MB total. Identical serialized states are skipped, and autosave never writes to the user project file.

Verification

  • npm run build
  • npm run lint -- --quiet
  • npm run test:frontend (4,629 passed, 1 skipped)
  • pre-commit run --files ...
  • Playwright browser verification in light and dark themes, including snapshot creation and restoring an earlier project name

Fixes #1522

Summary by CodeRabbit

  • New Features
    • Added Project History access through the Project menu.
    • Browse saved snapshots with dates and metadata.
    • Restore snapshots or discard recoverable sessions.
    • Automatically save unsaved changes for recovery.
    • Undo and redo now support whole-project restorations.
    • New projects and templates clear previous history.
    • Added localized messaging for history, restoration, and recovery actions.
    • Snapshot storage avoids duplicates and retains history within configured limits.

Copilot AI review requested due to automatic review settings August 1, 2026 02:55
@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This change adds IndexedDB project snapshots, debounced autosave, crash recovery, and a Project History interface. It also adds undoable whole-project restoration and connects history actions to the desktop shell.

Changes

Project history lifecycle

Layer / File(s) Summary
IndexedDB snapshot storage
apps/geolibre-desktop/src/lib/project-history-store.ts
Stores bounded project snapshots with metadata, duplicate detection, listing, deletion, and clearing.
Whole-project restore history
packages/core/src/store.ts, packages/core/src/index.ts, tests/undo-redo.test.ts
Adds restore entries and applies them through undo and redo when temporal history is exhausted. Tests cover project metadata, paths, and dirty state.
Autosave and recovery lifecycle
apps/geolibre-desktop/src/hooks/useProjectHistory.ts, apps/geolibre-desktop/src/lib/project-history-session.ts, apps/geolibre-desktop/src/hooks/useProjectFileActions.ts, apps/geolibre-desktop/src/components/layout/NewProjectDialog.tsx
Tracks sessions and explicit saves, autosaves dirty projects, detects recovery snapshots, restores snapshots, and clears history for new projects.
History and recovery interface
apps/geolibre-desktop/src/components/layout/ProjectHistoryDialog.tsx, apps/geolibre-desktop/src/components/layout/ProjectRecoveryDialog.tsx, apps/geolibre-desktop/src/i18n/locales/en.json, apps/geolibre-desktop/src/lib/ui-profile.ts, apps/geolibre-desktop/src/components/layout/toolbar/ProjectMenu.tsx
Adds localized history and recovery dialogs, restore and discard controls, and a profile-controlled Project History menu item.
Desktop shell and edit-menu wiring
apps/geolibre-desktop/src/components/layout/DesktopShell.tsx, apps/geolibre-desktop/src/components/layout/TopToolbar.tsx, apps/geolibre-desktop/src/components/layout/toolbar/EditMenu.tsx, apps/geolibre-desktop/src/hooks/useUndoRedoShortcuts.ts
Connects history callbacks, snapshot refresh, dialog state, recovery actions, and combined undo/redo availability.

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

Suggested reviewers: craun718

Poem

A rabbit saves each map with care,
Snapshots rest in digital lair.
When sessions end, recovery calls;
Restore or discard through guarded halls.
Undo and redo guide the way,
History keeps each change in play.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 19.05% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the PR's three primary changes: autosave, crash recovery, and project history.
Linked Issues check ✅ Passed The changes implement autosave, bounded IndexedDB snapshots, crash recovery, Project History restore, undoable restore, file-save isolation, and new-project cleanup [#1522].
Out of Scope Changes check ✅ Passed All changes support the linked issue's autosave, recovery, project-history, or undoable-restore objectives; no unrelated changes are evident [#1522].
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/issue-1522-project-history

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.

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

🔍 Cloudflare PR preview

Item Value
Site https://d21d7ae2.geolibre-preview.pages.dev
Demo app https://d21d7ae2.geolibre-preview.pages.dev/demo/
Commit e588758

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

🔍 GitHub Pages PR preview

Item Value
Site Deploy failed. See the job log.
Demo app Unavailable
Commit e588758

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Not ready to approve

Snapshot restore currently clears undo/redo history (so it isn’t undoable as described) and the history/recovery init path needs safer handling of IndexedDB/localStorage failures.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR adds a client-side autosave + recovery mechanism and a “Project → History” UI entry, integrating it into the GeoLibre desktop/web shell via a new IndexedDB-backed snapshot store and a React hook that debounces writes and exposes restore/discard actions.

Changes:

  • Add an IndexedDB “project history” ring buffer store with size/count caps and deduping.
  • Introduce useProjectHistory to autosave debounced snapshots, detect unclean sessions, and support recovery flows.
  • Add “History…” to the Project menu plus new History/Recovery dialogs and i18n strings.
File summaries
File Description
apps/geolibre-desktop/src/lib/ui-profile.ts Adds the project.history menu catalog entry.
apps/geolibre-desktop/src/lib/project-history-store.ts Implements IndexedDB persistence, snapshot retention limits, and CRUD helpers.
apps/geolibre-desktop/src/i18n/locales/en.json Adds strings for Project History + recovery prompt and the menu label.
apps/geolibre-desktop/src/hooks/useProjectHistory.ts Autosave debounce, recovery detection, and restore/discard logic wiring to the store.
apps/geolibre-desktop/src/components/layout/TopToolbar.tsx Threads a new onOpenProjectHistory callback into the toolbar → project menu.
apps/geolibre-desktop/src/components/layout/toolbar/ProjectMenu.tsx Adds the Project → History menu item and handler plumbing.
apps/geolibre-desktop/src/components/layout/ProjectRecoveryDialog.tsx Adds a recovery prompt dialog for restoring/discarding the latest autosave.
apps/geolibre-desktop/src/components/layout/ProjectHistoryDialog.tsx Adds a dialog to browse snapshots and restore one.
apps/geolibre-desktop/src/components/layout/DesktopShell.tsx Mounts the dialogs and hooks them up to shell state and menu actions.
Review details

Suppressed comments (2)

apps/geolibre-desktop/src/hooks/useProjectHistory.ts:75

  • This fire-and-forget delete operation doesn’t handle failures. If deleteProjectSnapshot() rejects, the rejection will be unhandled and the snapshot list may not refresh.
  const discardRecovery = useCallback(() => {
    if (recoverySnapshot) void deleteProjectSnapshot(recoverySnapshot.id).then(refresh);
    setRecoverySnapshot(null);

apps/geolibre-desktop/src/hooks/useProjectHistory.ts:47

  • This fire-and-forget clear operation doesn’t handle failures. If clearProjectSnapshots() rejects (e.g. transient IndexedDB issue), the rejection will be unhandled and refresh won’t run.
        if (state.projectPath === null && state.layers.length === 0) {
          void clearProjectSnapshots().then(refresh);
        }
  • Files reviewed: 9/9 changed files
  • Comments generated: 2
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment thread apps/geolibre-desktop/src/hooks/useProjectHistory.ts Outdated
Comment thread apps/geolibre-desktop/src/hooks/useProjectHistory.ts
Comment thread apps/geolibre-desktop/src/hooks/useProjectHistory.ts Outdated
Comment thread apps/geolibre-desktop/src/hooks/useProjectHistory.ts
Comment thread apps/geolibre-desktop/src/hooks/useProjectHistory.ts Outdated
Comment thread apps/geolibre-desktop/src/hooks/useProjectHistory.ts Outdated
Comment thread apps/geolibre-desktop/src/components/layout/ProjectRecoveryDialog.tsx Outdated
Comment thread apps/geolibre-desktop/src/lib/project-history-store.ts Outdated
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Code review

Bugs

  • useProjectHistory.ts:45-47 (high confidence): the "clear history on blank new project" check (projectPath === null && layers.length === 0) collides with restore(), which always calls loadProject(..., null, ...). Restoring any autosaved snapshot that happens to have zero layers wipes the entire project history instead of just applying that one snapshot.
  • useProjectHistory.ts:39-55 (high confidence): the store subscription has no selector, so it fires on every mutation, including setPointerCoords on each mousemove (a documented gotcha elsewhere in this codebase). While isDirty is true, moving the mouse over the map keeps resetting the 3-second autosave debounce, so autosave can be starved indefinitely during continuous interaction — exactly when crash protection matters most.
  • ProjectRecoveryDialog.tsx:25 (medium confidence): dismissing the dialog via Escape/outside-click is treated identically to clicking "Discard," permanently deleting the autosave snapshot rather than just closing the prompt.
  • useProjectHistory.ts:40-42 (medium confidence): LAST_SAVE_KEY is stamped on any isDirty: true→false transition (including loadProject/newProject, not just an explicit save), which can wrongly suppress the recovery prompt for genuinely unsaved work from a previous project.

Performance

  • project-history-store.ts:74-93 (low confidence): addProjectSnapshot reads back the full content of every stored snapshot (getAll(), up to 50 MB) on every autosave tick just to dedupe/evict; metadata could be kept separate from the large content blob.
  • Same range: the eviction loop uses continue instead of break once the size/count budget is exceeded, which can retain a non-contiguous set of snapshots rather than a clean "N most recent that fit" window.

Quality

  • useProjectHistory.ts:64-76 (medium confidence): restore()'s parseProject call isn't wrapped in error handling (unlike the addProjectSnapshot path), and discardRecovery/the clear-history branch omit the .catch used elsewhere, so IndexedDB failures surface inconsistently (some silently swallowed via console.error, others as unhandled rejections).

Security

  • Nothing found. Content is only ever read back through JSON.parse/parseProject and rendered via i18next interpolation (not dangerouslySetInnerHTML); IndexedDB access is same-origin and unauthenticated by design.

CLAUDE.md

  • No violations found: new user-facing strings go through t()/en.json, RTL-safe Tailwind classes (me-2) are used, and the menu entry is wired through the checked-in ui-profile.ts catalog rather than hand-editing a generated file.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 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 `@apps/geolibre-desktop/src/components/layout/DesktopShell.tsx`:
- Around line 2342-2352: Update useProjectHistory.restore, used by both
ProjectHistoryDialog and ProjectRecoveryDialog, to capture the current canonical
project state before calling loadProject, then register a single undoable
operation that can restore that pre-restore state. Preserve the existing
snapshot restoration behavior while ensuring both history and crash-recovery
restores support undo without losing unsaved work.

In `@apps/geolibre-desktop/src/components/layout/ProjectHistoryDialog.tsx`:
- Around line 53-56: Update the zoom formatting in the ProjectHistoryDialog
summary around the snapshot rendering to use the active locale’s number
formatter instead of zoom.toFixed(1), while preserving one decimal place and the
existing translation parameters.

In `@apps/geolibre-desktop/src/hooks/useProjectHistory.ts`:
- Around line 39-42: Update useProjectHistory and the project-writing flow to
stop deriving LAST_SAVE_KEY from the isDirty transition subscription. Introduce
or reuse an explicit markSaved() callback, and invoke it only after the project
file is successfully written; ensure newProject() and project loads do not
update LAST_SAVE_KEY.

In `@apps/geolibre-desktop/src/lib/project-history-store.ts`:
- Around line 69-72: Update addProjectSnapshot so the size-limit rejection when
size exceeds MAX_SNAPSHOT_BYTES surfaces an actionable reason instead of
silently returning false; at minimum log the rejection with the measured and
allowed sizes, or return a reason code that useProjectHistory.ts can distinguish
and present to the user while preserving normal duplicate-skip behavior.
- Around line 86-93: Update the retention loop that builds keepIds from the
newest-first retained list to break when MAX_SNAPSHOTS or MAX_TOTAL_BYTES would
be exceeded, rather than continue, so older entries cannot fill gaps after a
rejected snapshot.
🪄 Autofix (Beta)

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: ASSERTIVE

Plan: Pro Plus

Run ID: 15b74572-72fb-460e-bcb5-b8df2343be6f

📥 Commits

Reviewing files that changed from the base of the PR and between 97825e8 and 6230b2f.

📒 Files selected for processing (9)
  • apps/geolibre-desktop/src/components/layout/DesktopShell.tsx
  • apps/geolibre-desktop/src/components/layout/ProjectHistoryDialog.tsx
  • apps/geolibre-desktop/src/components/layout/ProjectRecoveryDialog.tsx
  • apps/geolibre-desktop/src/components/layout/TopToolbar.tsx
  • apps/geolibre-desktop/src/components/layout/toolbar/ProjectMenu.tsx
  • apps/geolibre-desktop/src/hooks/useProjectHistory.ts
  • apps/geolibre-desktop/src/i18n/locales/en.json
  • apps/geolibre-desktop/src/lib/project-history-store.ts
  • apps/geolibre-desktop/src/lib/ui-profile.ts

Comment thread apps/geolibre-desktop/src/components/layout/DesktopShell.tsx
Comment thread apps/geolibre-desktop/src/hooks/useProjectHistory.ts
Comment thread apps/geolibre-desktop/src/lib/project-history-store.ts Outdated
Comment thread apps/geolibre-desktop/src/lib/project-history-store.ts
Comment thread apps/geolibre-desktop/src/hooks/useProjectHistory.ts Outdated
Comment thread apps/geolibre-desktop/src/lib/project-history-store.ts Outdated
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Code review

Bugs

  • useProjectHistory.ts:55 — the crash-recovery heuristic assumes pagehide always fires on a clean exit, but this hook runs unconditionally across the web, Tauri desktop, and embedded builds. The sibling useBeforeUnloadGuard hook explicitly disables itself for both isTauri() and isEmbedded(), noting a desktop window close "does not raise a useful web prompt" and the embed host "owns the page lifecycle." If pagehide doesn't fire on a real Tauri window close (no onCloseRequested/window-event wiring exists to confirm it does), every normal desktop quit would leave the session marked "open", triggering a spurious "Recover unsaved work?" prompt on the next launch, and the same dialog could pop up unexpectedly inside the embedded Jupyter widget. Confidence: medium.
  • project-history-store.ts:44-65indexedDB.open has no onblocked handler, so if another tab/context holds the DB open across a version bump, the open request (and everything awaiting it) can hang indefinitely with no timeout or surfaced error. Low impact today since DB_VERSION is fixed, but a real gap for multi-tab web usage or future schema bumps. Confidence: low-medium.
  • (Minor, not inline) project-history-session.ts — the "open"/"closed" session marker is a single shared localStorage key, so concurrent tabs/windows of the web build can clobber each other's session state, causing false-positive or false-negative recovery prompts. Confidence: low.

Security

  • None found. crypto.subtle.digest/crypto.randomUUID usage in project-history-store.ts matches an existing pattern already used elsewhere in the codebase (e.g. plugin-integrity.ts), so it's not a new risk introduced by this PR, though it does depend on a secure context.

Performance

  • No significant issues. The autosave path debounces 3s, dedups identical serialized states via a content hash, and prunes by count/size; the SHA-256 hashing and full JSON parseProject run per autosave but are bounded by the 10 MB per-snapshot cap.

Quality

  • The undo/redo interplay between the new whole-project "restore" history and zundo's temporal history (packages/core/src/store.ts) is a genuinely subtle piece of state machinery, but it holds up under review — the applyingProjectRestoreHistory guard correctly prevents clearHistory() from wiping the restore stack during an undo/redo of a restore itself.
  • ProjectHistoryDialog.tsx re-creates Intl.DateTimeFormat/Intl.NumberFormat instances per snapshot per render — negligible at the 20-snapshot cap, not worth flagging as a real issue.

CLAUDE.md

  • New user-facing strings are added only to en.json and go through t(), consistent with the i18n convention (other locales fall back automatically). No RTL-relevant physical positioning classes were introduced. No violations found.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

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

⚠️ Outside diff range comments (2)
apps/geolibre-desktop/src/components/layout/ProjectHistoryDialog.tsx (1)

64-66: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Keep the history dialog open when restoration fails.

onOpenChange(false) runs after every onRestore(snapshot) call. useProjectHistory.restore catches parse and load errors and only logs them, so an invalid snapshot closes the dialog without an error message or retry path. Make onRestore return a success result, or expose a restore error, and close the dialog only after success.

🤖 Prompt for 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.

In `@apps/geolibre-desktop/src/components/layout/ProjectHistoryDialog.tsx` around
lines 64 - 66, Update the ProjectHistoryDialog restore flow so onRestore reports
whether restoration succeeded, and call onOpenChange(false) only after a
successful restore. Preserve the dialog’s open state when
useProjectHistory.restore encounters parse or load errors, allowing the user to
retry or see the existing error handling.
apps/geolibre-desktop/src/lib/project-history-store.ts (1)

42-67: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Add an onblocked handler in openDatabase.

indexedDB.open(DB_NAME, 2) runs a version-upgrade request. If an older database connection remains open in another tab/window, onblocked fires before onupgradeneeded; without a handler, this request can miss both onsuccess and onerror. This blocks every awaited call site: listProjectSnapshots, addProjectSnapshot, deleteProjectSnapshot, clearProjectSnapshots, ProjectHistoryDialog, and NewProjectDialog.

Add a handler that rejects the existing promise so callers can fail fast when the version upgrade is blocked.

🔒 Proposed fix
 function openDatabase(): Promise<IDBDatabase> {
   return new Promise((resolve, reject) => {
     const request = indexedDB.open(DB_NAME, DB_VERSION);
+    request.onblocked = () =>
+      reject(new Error("Could not open project history: blocked by another open connection."));
     request.onupgradeneeded = () => {
🤖 Prompt for 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.

In `@apps/geolibre-desktop/src/lib/project-history-store.ts` around lines 42 - 67,
Update openDatabase to register an onblocked handler on the IndexedDB open
request and reject the existing promise with an appropriate error when the
version upgrade is blocked, while preserving the current onsuccess, onerror, and
upgrade behavior.
🤖 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 `@apps/geolibre-desktop/src/hooks/useProjectHistory.ts`:
- Around line 81-99: Update the useProjectHistory hook’s restore callback and
return value to expose restore failures instead of only logging them. Add
restoreError state, set it with a user-facing message when parseProject or
loadProject throws, clear it before a new restore and after a successful
restore, and return it so recovery and Project History callers can display the
failure.

In `@packages/core/src/store.ts`:
- Around line 2208-2224: Wrap the loadProject calls in both the undo() and
redo() project-restore branches with try/catch, logging failures via
console.error consistently with useProjectHistory.ts restore(). Ensure thrown
loadProject errors are handled without escaping undo()/redo(), while preserving
the existing finally cleanup and history/state handoff behavior for successful
restores.
- Around line 2208-2224: Clear the module-level projectRestoreRedo pointer when
a new edit is committed after project-restore undo, before redo can reuse the
stale entry. Update the normal temporal commit path, not the loadProject restore
flow guarded by applyingProjectRestoreHistory, so the pointer is preserved
during undo restoration and invalidated only by the next user edit.

In `@tests/undo-redo.test.ts`:
- Around line 11-17: Add a regression case in the undo/redo tests covering a
restore undo followed by a normal store edit that adds an entry to
temporal.pastStates before redo(); assert that redo preserves the intervening
edit rather than applying the stale after snapshot. Reuse the existing restore
setup and helpers such as registerProjectRestoreHistory, undo, redo, and
useAppStore, while keeping the direct undo-then-redo test unchanged.

---

Outside diff comments:
In `@apps/geolibre-desktop/src/components/layout/ProjectHistoryDialog.tsx`:
- Around line 64-66: Update the ProjectHistoryDialog restore flow so onRestore
reports whether restoration succeeded, and call onOpenChange(false) only after a
successful restore. Preserve the dialog’s open state when
useProjectHistory.restore encounters parse or load errors, allowing the user to
retry or see the existing error handling.

In `@apps/geolibre-desktop/src/lib/project-history-store.ts`:
- Around line 42-67: Update openDatabase to register an onblocked handler on the
IndexedDB open request and reject the existing promise with an appropriate error
when the version upgrade is blocked, while preserving the current onsuccess,
onerror, and upgrade behavior.
🪄 Autofix (Beta)

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: ASSERTIVE

Plan: Pro Plus

Run ID: 54c2e177-aaae-452b-bab5-d4e14bc8b842

📥 Commits

Reviewing files that changed from the base of the PR and between 6230b2f and 9e981a4.

📒 Files selected for processing (12)
  • apps/geolibre-desktop/src/components/layout/DesktopShell.tsx
  • apps/geolibre-desktop/src/components/layout/NewProjectDialog.tsx
  • apps/geolibre-desktop/src/components/layout/ProjectHistoryDialog.tsx
  • apps/geolibre-desktop/src/components/layout/ProjectRecoveryDialog.tsx
  • apps/geolibre-desktop/src/components/layout/toolbar/EditMenu.tsx
  • apps/geolibre-desktop/src/hooks/useProjectFileActions.ts
  • apps/geolibre-desktop/src/hooks/useProjectHistory.ts
  • apps/geolibre-desktop/src/lib/project-history-session.ts
  • apps/geolibre-desktop/src/lib/project-history-store.ts
  • packages/core/src/index.ts
  • packages/core/src/store.ts
  • tests/undo-redo.test.ts

Comment thread apps/geolibre-desktop/src/hooks/useProjectHistory.ts
Comment thread packages/core/src/store.ts
Comment thread tests/undo-redo.test.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@apps/geolibre-desktop/src/lib/project-history-store.ts`:
- Around line 69-74: Update the IndexedDB request handlers in the
project-history initialization flow to track whether the Promise has already
settled. When `onblocked` rejects and `onsuccess` later fires, close
`request.result` instead of resolving; retain normal resolution when success
occurs first.
🪄 Autofix (Beta)

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: ASSERTIVE

Plan: Pro Plus

Run ID: 3907010f-d897-4eaf-88e4-1bb6bd2f0446

📥 Commits

Reviewing files that changed from the base of the PR and between 9e981a4 and b8c55d3.

📒 Files selected for processing (2)
  • apps/geolibre-desktop/src/hooks/useProjectHistory.ts
  • apps/geolibre-desktop/src/lib/project-history-store.ts

Comment thread apps/geolibre-desktop/src/lib/project-history-store.ts
Comment thread apps/geolibre-desktop/src/components/layout/toolbar/EditMenu.tsx
Comment thread apps/geolibre-desktop/src/components/layout/DesktopShell.tsx
Comment thread apps/geolibre-desktop/src/hooks/useProjectHistory.ts Outdated
Comment thread apps/geolibre-desktop/src/lib/project-history-session.ts
Comment thread apps/geolibre-desktop/src/lib/project-history-store.ts Outdated
Comment thread apps/geolibre-desktop/src/hooks/useProjectHistory.ts Outdated
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

All six inline comments posted successfully.

Code review

Bugs

  • Ctrl/Cmd+Z and Ctrl/Cmd+Shift+Z no-op after a Project History restore with no prior temporal edits: useUndoRedoShortcuts.ts (unchanged by this PR) still gates the shortcuts on useAppStore.temporal.getState().pastStates/futureStates.length, not on the new canUndoProjectRestore()/canRedoProjectRestore() that EditMenu.tsx now checks — the Edit menu shows Undo enabled and works, but the shortcut it advertises does nothing. High confidence.
  • restore() in useProjectHistory.ts always passes null as the path to loadProject, detaching the restored project from its original file — the next Save silently becomes Save As, even when just browsing history on an already-saved file (the "undo the restore" path correctly keeps the original path; only the forward restore drops it). Medium confidence.

Security

  • None found. The IndexedDB access, snapshot serialization, and localStorage keys are all local/same-origin with no user-controlled paths or injected content.

Performance

  • useProjectHistory's refresh() calls listProjectSnapshots(), which reads the full snapshot content (up to 50MB total across 20 entries) and runs after every successful autosave (~every 3s while editing), even when no history UI is visible — despite the module already maintaining a lighter METADATA_STORE_NAME for cheap listing that isn't reused here. Medium confidence.

Quality

  • The crash-recovery SESSION_KEY in localStorage and the IndexedDB snapshot ring buffer are both global (not per-tab, not per-project). A clean close in one tab can suppress the crash-recovery prompt for a still-open tab with real unsaved work, and switching to a different existing project (not "New") leaves an unrelated project's snapshots in the shared History/quota. Medium confidence.
  • addProjectSnapshot calls crypto.subtle.digest and crypto.randomUUID() unconditionally; both require a secure context (HTTPS/localhost). On an HTTP-only nginx/Docker deployment this throws every autosave cycle, silently killing the whole feature (caught only by console.error). Medium confidence.
  • restoreError is shared state rendered by both ProjectHistoryDialog and ProjectRecoveryDialog; a failed restore in one can leave a stale error visible in the other since it isn't dialog-scoped or cleared on open/close. Low-medium confidence.

CLAUDE.md

  • No violations found: en.json is updated as the translation source of truth, RTL-safe Tailwind classes aren't needed here (no directional spacing added beyond existing me-2 usage), and no other guideline in CLAUDE.md applies to these files.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@apps/geolibre-desktop/src/components/layout/DesktopShell.tsx`:
- Around line 2346-2351: Update the DesktopShell dialog flow and
useProjectHistory integration so the shared restoreError is cleared whenever
either ProjectHistoryDialog or ProjectRecoveryDialog closes, discards, or is
dismissed. Add and pass a clearRestoreError callback, wiring it to every
relevant close/discard handler rather than leaving stale error state for the
other dialog.

In `@apps/geolibre-desktop/src/hooks/useProjectHistory.ts`:
- Around line 102-106: Update the restore error handling in useProjectHistory so
setRestoreError stores a translated, non-empty projectHistory message via t(),
rather than exposing error.message or using the literal fallback. Keep the
original exception available only to console.error, and ensure
ProjectHistoryDialog and ProjectRecoveryDialog render the translated
restoreError without duplicating fallback logic.
🪄 Autofix (Beta)

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: ASSERTIVE

Plan: Pro Plus

Run ID: c68879a7-a03a-4239-a16c-2ea19689ff49

📥 Commits

Reviewing files that changed from the base of the PR and between b8c55d3 and 503ae18.

📒 Files selected for processing (6)
  • apps/geolibre-desktop/src/components/layout/DesktopShell.tsx
  • apps/geolibre-desktop/src/components/layout/ProjectHistoryDialog.tsx
  • apps/geolibre-desktop/src/components/layout/ProjectRecoveryDialog.tsx
  • apps/geolibre-desktop/src/hooks/useProjectHistory.ts
  • packages/core/src/store.ts
  • tests/undo-redo.test.ts

Comment thread apps/geolibre-desktop/src/components/layout/DesktopShell.tsx
Comment thread apps/geolibre-desktop/src/hooks/useProjectHistory.ts Outdated
}
}

export async function clearProjectSnapshots(): Promise<void> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: clearProjectSnapshots unconditionally clears the entire snapshots/snapshot-metadata object stores — every project's autosave history, not just the one being abandoned. It's called from NewProjectDialog.tsx whenever the user creates a blank project or starts from a template, so opening "New Project" (or a template) wipes the Project History/recovery data for every other project the user has ever worked on in this browser/profile, including unrelated saved projects that still have valid autosaves. Since ProjectHistorySnapshot already carries a projectKey, this should instead delete only entries matching the current (soon-to-be-replaced) project key, e.g. a clearProjectSnapshots(projectKey: string) that filters listProjectSnapshotMetadata()/getAll() and deletes matching ids, mirroring how listProjectSnapshots(projectKey) already filters.

Confidence: medium-high — the intent stated in the PR description ("clear recovery history when starting a blank new project") is about the abandoned draft, not a global wipe, and the multi-project design elsewhere (per-key filtering in listProjectSnapshots/addProjectSnapshot) suggests this was meant to be scoped too.

Comment on lines +50 to +55
const entries = await listProjectSnapshots();
setSnapshots(entries.filter((entry) => entry.projectKey === currentProjectKey()));
if (crashRecoveryEnabled) {
const previousSession = readProjectSessionState();
const lastSave = readLastExplicitProjectSave();
const latest = entries[0];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: entries here comes from listProjectSnapshots() called with no projectKey, so it contains snapshots for every project stored in IndexedDB, sorted by createdAt across all of them. latest = entries[0] is therefore the globally most-recent autosave, not necessarily one belonging to the project that's about to open (currentProjectKey(), used two lines above to filter snapshots). If another project was autosaved more recently than the current project's own last snapshot, the crash-recovery dialog will offer to restore a completely unrelated project's content — mislabeled as "GeoLibre did not close cleanly" for the project the user is currently in. Restoring it would then overwrite the current session with the wrong project's data.

This compounds with recordExplicitProjectSave/readLastExplicitProjectSave also being global (not per-project), so lastSave doesn't correctly gate this either.

Fix: filter entries to entry.projectKey === currentProjectKey() (like the snapshots filter right above) before taking entries[0].

Confidence: medium-high.

const project = parseProject(content);
const contentHash = hashContent(content);
const existing = await listProjectSnapshotMetadata();
if (existing[0]?.contentHash === contentHash) return "duplicate";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug (minor): the duplicate check compares the new snapshot's hash against existing[0], the globally most-recently-created snapshot across all projects (since listProjectSnapshotMetadata() isn't filtered by projectKey). When more than one project is being autosaved (e.g. two tabs, or switching projects), this can miss real duplicates for the current project — if some other project autosaved more recently, an unchanged repeat save of this project's state won't be recognized as identical to its own last snapshot, so it gets written again anyway, consuming a slot in the shared 20-entry/50MB ring buffer and potentially evicting another project's older snapshot unnecessarily. The PR description promises "identical serialized states are skipped," which doesn't hold across concurrent/multi-project use.

Suggest comparing against the latest entry filtered to projectKey instead of existing[0].

Confidence: medium.

);
return "too-large";
}
const project = parseProject(content);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quality/perf nit: addProjectSnapshot re-parses the just-serialized content string back into a GeoLibreProject purely to read name/layers.length/basemapStyleUrl/mapView for the metadata record. The only caller (useProjectHistory.ts) already has the pre-serialization GeoLibreProject object from buildProjectSnapshot(...) before calling serializeProject(...) — this forces an extra parse of a potentially multi-MB JSON string on every autosave (debounced to at most once per 3s, so not hot-path-critical, but still avoidable). Consider accepting the already-built project object (or just the needed metadata fields) alongside the serialized string, instead of parsing it back out.

Confidence: low — purely a minor inefficiency, not a correctness issue.

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Code review

Bugs

  • apps/geolibre-desktop/src/lib/project-history-store.ts:227clearProjectSnapshots() clears the entire IndexedDB snapshot store globally, but it's invoked from NewProjectDialog.tsx on every "New Project"/template creation. This wipes Project History and crash-recovery data for all projects on the device, not just the abandoned draft. Confidence: medium-high.
  • apps/geolibre-desktop/src/hooks/useProjectHistory.ts:50-57 — Crash-recovery detection takes entries[0] from an unfiltered (all-projects) snapshot list as "the latest autosave," instead of filtering to currentProjectKey() as the snapshots state does one line above. This can surface (and let the user restore) an unrelated project's autosave as if it were recovering the current project's unclean session. Compounded by recordExplicitProjectSave/readLastExplicitProjectSave also being global rather than per-project. Confidence: medium-high.
  • apps/geolibre-desktop/src/lib/project-history-store.ts:173 — The duplicate-autosave check compares against the globally most-recent snapshot (existing[0]) rather than the most recent snapshot for the current projectKey, so "identical serialized states are skipped" (per PR description) can fail to hold when more than one project is being autosaved, causing needless duplicate writes and ring-buffer churn. Confidence: medium.

Security

  • Nothing found. Snapshot content stays in IndexedDB, restore paths go through parseProject, and no user input reaches injection-sensitive sinks.

Performance

  • apps/geolibre-desktop/src/lib/project-history-store.ts:170addProjectSnapshot re-parses the just-serialized project JSON back into an object solely to extract metadata the caller already had before serializing. Minor, debounce-bounded inefficiency, not correctness-affecting. Confidence: low.

Quality

  • The undo/redo project-restore integration in packages/core/src/store.ts (the applyingProjectRestoreHistory flag guarding clearHistory() re-entrancy, and clearing only projectRestoreRedo on new temporal edits) is well-reasoned and the added tests in tests/undo-redo.test.ts cover the interesting cases. No issues found there.
  • project-history-session.ts's localStorage read-modify-write for session state (readSessions/markProjectSession) isn't atomic across tabs, so concurrent tabs could race and drop each other's "open" mark. Low impact given it's only a crash heuristic. Not filed inline given its low severity.

CLAUDE.md

  • New user-facing strings correctly go through t() with entries in en.json; no adherence issues found for this change.

@giswqs
giswqs enabled auto-merge (squash) August 1, 2026 03:52

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

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

⚠️ Outside diff range comments (2)
apps/geolibre-desktop/src/hooks/useProjectHistory.ts (1)

50-58: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Scope the crash-recovery snapshot to the current project.

Line 51 filters entries by currentProjectKey() before showing them in the History list. latest at line 55 comes from the same, unfiltered entries array, sorted newest-first across every project. If the user worked on more than one project in the same browser session before an unclean shutdown, latest can belong to a different project than the one currently loaded.

If the recovery prompt is then accepted, restore(latest) calls loadProject, replacing the currently open project's content with an unrelated project's autosave. The recovery dialog does show the snapshot's name, but a user acting on the prompt without reading it closely can overwrite unrelated, unrecovered work.

Filter entries by currentProjectKey() before picking latest, the same way line 51 already does for the visible list.

🐛 Proposed fix
-          const latest = entries[0];
+          const latest = entries.find((entry) => entry.projectKey === currentProjectKey());
🤖 Prompt for 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.

In `@apps/geolibre-desktop/src/hooks/useProjectHistory.ts` around lines 50 - 58,
Scope crash recovery to the current project by filtering entries with
currentProjectKey() before selecting latest in the recovery block. Reuse the
project-filtered snapshots for both setSnapshots and latest, while preserving
the existing newest-first and recovery eligibility checks.
apps/geolibre-desktop/src/lib/project-history-store.ts (1)

110-124: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift

Avoid loading full snapshot content just to list/filter by project.

listProjectSnapshots reads every stored snapshot's full content field via getAll() on STORE_NAME (up to 10 MB each, 50 MB total), then filters and sorts in JavaScript. useProjectHistory.ts calls this function on hook initialization and again after every successful autosave (roughly every 3 seconds while the project stays dirty), so the entire ring buffer's content is deserialized into memory on a steady cadence, even when no history UI is visible.

listProjectSnapshotMetadata() already exists in this file and is used internally by addProjectSnapshot for the same kind of read (dedup/retention) without touching content. Expose an equivalent metadata-only listing for the browsing/display path, and fetch full content only when a specific snapshot is restored.

♻️ Suggested direction
-export async function listProjectSnapshots(projectKey?: string): Promise<ProjectHistorySnapshot[]> {
-  if (!available()) return [];
-  const db = await openDatabase();
-  try {
-    const transaction = db.transaction(STORE_NAME, "readonly");
-    const snapshots = await requestResult(
-      transaction.objectStore(STORE_NAME).getAll() as IDBRequest<ProjectHistorySnapshot[]>,
-    );
-    return snapshots
-      .filter((snapshot) => projectKey === undefined || snapshot.projectKey === projectKey)
-      .sort((a, b) => b.createdAt.localeCompare(a.createdAt));
-  } finally {
-    db.close();
-  }
-}
+export async function listProjectSnapshotSummaries(
+  projectKey?: string,
+): Promise<ProjectHistoryMetadata[]> {
+  const metadata = await listProjectSnapshotMetadata();
+  return metadata.filter(
+    (entry) => projectKey === undefined || entry.projectKey === projectKey,
+  );
+}

Then have useProjectHistory.ts render from the metadata list and load full content on demand (e.g. a getProjectSnapshot(id) helper) only when the user restores a specific entry.

🤖 Prompt for 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.

In `@apps/geolibre-desktop/src/lib/project-history-store.ts` around lines 110 -
124, Change listProjectSnapshots to return metadata without deserializing
snapshot content, reusing the existing listProjectSnapshotMetadata pattern for
project filtering and newest-first sorting. Add or expose a getProjectSnapshot
helper that retrieves full content by snapshot ID, then update useProjectHistory
to render metadata and fetch the selected snapshot only during restore.
🤖 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.

Outside diff comments:
In `@apps/geolibre-desktop/src/hooks/useProjectHistory.ts`:
- Around line 50-58: Scope crash recovery to the current project by filtering
entries with currentProjectKey() before selecting latest in the recovery block.
Reuse the project-filtered snapshots for both setSnapshots and latest, while
preserving the existing newest-first and recovery eligibility checks.

In `@apps/geolibre-desktop/src/lib/project-history-store.ts`:
- Around line 110-124: Change listProjectSnapshots to return metadata without
deserializing snapshot content, reusing the existing listProjectSnapshotMetadata
pattern for project filtering and newest-first sorting. Add or expose a
getProjectSnapshot helper that retrieves full content by snapshot ID, then
update useProjectHistory to render metadata and fetch the selected snapshot only
during restore.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 186e15dc-639a-488c-babd-968f6435703c

📥 Commits

Reviewing files that changed from the base of the PR and between 503ae18 and e588758.

📒 Files selected for processing (8)
  • apps/geolibre-desktop/src/components/layout/DesktopShell.tsx
  • apps/geolibre-desktop/src/hooks/useProjectHistory.ts
  • apps/geolibre-desktop/src/hooks/useUndoRedoShortcuts.ts
  • apps/geolibre-desktop/src/i18n/locales/en.json
  • apps/geolibre-desktop/src/lib/project-history-session.ts
  • apps/geolibre-desktop/src/lib/project-history-store.ts
  • packages/core/src/store.ts
  • tests/undo-redo.test.ts

@giswqs
giswqs merged commit ecef212 into main Aug 1, 2026
16 checks passed
@giswqs
giswqs deleted the fix/issue-1522-project-history branch August 1, 2026 03:53
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.

Autosave, crash recovery, and a Project History dialog

2 participants