Skip to content

Native SAF export/backup/restore (Android OOM fix) + export/import fixes, #330, #116#367

Open
Pento95 wants to merge 4 commits into
AventurasTeam:masterfrom
Pento95:feat/android-oom-memory
Open

Native SAF export/backup/restore (Android OOM fix) + export/import fixes, #330, #116#367
Pento95 wants to merge 4 commits into
AventurasTeam:masterfrom
Pento95:feat/android-oom-memory

Conversation

@Pento95

@Pento95 Pento95 commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Apologies up front 🙏

This is a big, multi-topic PR — a "god-PR", and I'm sorry for that. The changes grew out of one investigation (Android OOM on export/backup) and pulled in several adjacent fixes I had queued locally. I know a bundle like this is harder to review than a focused branch, so I've documented each area separately below and I'm happy to split it into smaller PRs if you'd prefer — just say the word.


Overview

The core of this work fixes Android OutOfMemory crashes during export, backup, restore and import, and makes those flows fast. It also folds in two issue fixes and a few UX/quality improvements.

1. Android OOM: native, streamed export / backup / restore

Problem. Exports and backups routed hundreds of MB (base64 images, the full SQLite DB) through the JS/IPC bridge as strings. On Android's constrained WebView heap this reliably OOM-crashed.

Fix. All heavy file I/O now happens natively in Rust and streams file-to-file — nothing large ever lives in the JS heap or crosses the bridge:

  • backup_database, export_story_avt, export_single_image, export_images_zip write directly to the chosen destination.
  • On desktop the destination is the real path from the save dialog.
  • On Android the destination is the SAF content:// URI from the save dialog, opened natively via the fs plugin's content-URI file descriptor (ContentResolver). Writes stream in ~8 KB blocks, so peak memory is a single buffer regardless of file size.
  • Restore mirrors this: the picked content:// backup is streamed into a temp file in the app's internal cache dir, then restored natively.
  • The .avt exporter fills each image's base64 from SQLite on the native side, so image bytes never touch the WebView heap.

Perf. Backup DB compression dropped from Deflate level 6 → level 1 (the DB is mostly already-compressed base64 PNGs, so level 1 recovers the base64 bloat at a fraction of the CPU), and the .avt exporter now pulls all images in one streamed query instead of one query per image.

2. Native .avt import — the OOM's other half

The known limitation this PR originally shipped with ("importing an extremely large .avt still parses JSON in the WebView") turned out to be a hard failure, not just memory pressure, and it is now fixed.

Problem. The Library's Import button read the picked file with an <input type="file"> and file.text(). That has to materialise the whole .avt as a JS string, and V8 caps a string at 2^29 - 24 bytes (~512 MiB). A real 546 MB export exceeded it by 1.8% and failed with a misleading "not a valid JSON file" — the file was intact; the read never delivered it. No amount of RAM helps: the ceiling is structural.

Fix. Import now goes through the native picker into a streaming Rust reader, mirroring the exporter's split — JS owns the structure, Rust owns the bytes. Id remapping, insertion order and foreign keys stay in TypeScript where they are tested; only the payloads move.

Two passes, both streaming:

  1. avt_read_light returns the JSON with every imageData removed (serde's IgnoredAny skips each base64 without allocating it). JS parses that and runs the ordinary import.
  2. avt_import_images re-reads the file and streams each payload straight into SQLite, using the small old-image-id → new-entry-id table JS produced.

Re-reading is deliberate: a staging table would need a migration and would leak orphan rows if an import died half way. Peak memory is one image regardless of file size — measured at 16 kB vs 108 MB for a naive parse of a 100 MB file (cargo test --lib avt_import -- --ignored --nocapture).

The structure and the images cannot share a transaction (the native side writes over its own connection), so a failed image pass deletes the half-imported story rather than leaving one with its pictures silently missing.

Verified on-device: the 546 MB .avt that previously failed now imports in ~1 minute.

3. Export/import robustness

  • New errMessage() helper: Tauri command rejections arrive as plain strings, so the old e instanceof Error ? … : 'Unknown error' discarded the real (often precise) native message. Now surfaced everywhere export/import can fail.
  • Export toasts now report failures instead of failing silently (the .avt gather step used to run outside the try/catch).

4. Closes #330 — Pollinations image models missing

The provider filtered out models flagged paid_only, hiding valid image models from the list. Filter removed so all image-capable models appear.

The "Include paid models" toggle is gone with it: the API already filters by tier server-side from the API key, so the toggle was a client-side duplicate that was inert wherever the key was actually sent (images) and misleading where it wasn't (text). The text model fetch now sends the key too, so the list reflects what the key can really generate.

5. Closes #116 — Adjustable chapter summary length

Chapter summary length is now configurable rather than fixed.

6. Generation info popover

Narration entries now have an info popover showing the model / profile / effort / timestamp used to generate them.


Review fixes

From the Gemini and CodeRabbit reviews, plus bugs found while verifying their findings. Full reasoning in this comment.

Restore could destroy the database. File::create truncated the live aventura.db before extraction succeeded, so a CRC error, a truncated archive or a full disk left it in pieces — and the safety copy's failure was only eprintln!'d, with nothing ever restoring from it. The DB is now staged and validated first, then swapped in by renaming the old file aside: atomic, and free in disk space, which matters when the DB is ~270 MB on a phone. Covered by 6 Rust tests, checked to fail against the old logic.

Backups could silently omit recent data. When VACUUM INTO failed, the code fell back to archiving the live aventura.db. The DB runs in WAL mode (verified: header bytes 02 02), so recent commits can live in aventura.db-wal — the fallback could ship a stale backup, the worst failure mode for a backup, since it only surfaces at restore time. A failed snapshot is now a hard error.

Gallery could re-create the OOM this PR fixes. imageDataCache was cleared on refresh but not on story change, and had no eviction: lazy loading bounds the peak at open, not the total. It is now an LRU with a byte budget, cleared on story change. Failed loads render a Retry instead of a permanent spinner.

Staged SAF restore temps were never deleted despite the doc comment claiming otherwise — full copies of the backup, hundreds of MB. They are now swept on the way in: a cleanup after the restore would never run, because the app calls exit(0) on success.

Bugs found while reviewing the reviews

  • Checkpoint snapshots kept stale chapter ids. Checkpoints are imported before chapters, so their snapshots' remapping found nothing in the id map and silently kept the export's original ids — a rollback on an imported story would restore chapters that do not exist. Chapters are now pre-mapped.
  • Every export was stamped v1.7.0 while the importer checked for v1.8.0, so re-importing our own file always warned that the background image would not be restored — while it was. (This versions the .avt file format; it is unrelated to the app version, which the code never said. It does now.)
  • Android file pickers dropped their filters. The dialog plugin maps each extension to a MIME type with MimeTypeMap.getMimeTypeFromExtension, which returns null for anything Android does not know — including our own avt and the * wildcard — and discards it silently. ['avt', 'json', '*'] became "JSON only", grey-ing out every .avt; the "All Files" escape hatch never worked. Confirmed on-device for restore too: backup.zip (1), the exact duplicate-name case the fallback was written for, was unselectable. Filters are now omitted on Android, where they could only lie.
  • A dead oldToNewId.set(image.id, …) wrote image ids into the same map used for entry lookups two lines below — harmless today, corrupting on a collision.

Not applied (false positives)

  • "Potential null pointer dereference on memoryConfig"story.memoryConfig is a store getter that coalesces to DEFAULT_MEMORY_CONFIG; it never returns null.
  • "Keep one source of truth for the memory configuration types"services/ai/index.ts imports those types from $lib/types rather than declaring them; there is one declaration of each.

Refactor

importFromContent was a single ~490-line function holding the most delicate logic in the project (id remapping, COW overridesId, topological branch insertion). It is now services/import/, split into validate / idMaps / structure / images, with the image payloads behind a strategy — inline for sync, native for a picked file. Both entry points share one implementation of the structure.

Adds vitest and the first frontend tests. 33 tests, written against the import's public entry point before the refactor so it could be judged by "these still pass", and checked by mutating the source to confirm they fail on a regression (one of them didn't, and was fixed). Plus 14 Rust tests, three writing to a real SQLite database.

Testing

  • npm run check, npm run lint, cargo fmt, cargo clippy clean. 33 frontend tests, 14 Rust tests.
  • On-device (Android, arm64): verified export .avt (~486 MB, valid JSON confirmed byte-for-byte), backup (~363 MB, well compressed), restore (app restarts cleanly), a gallery of 800+ images, Pollinations model fetch, and the 546 MB .avt import that previously failed.

Known limitations

  • Sync inherits the same ceiling. SyncModal receives a story as a string over the network, so there is no file to hand to Rust and a synced story over ~512 MiB would fail the same way. That is a change to the sync transport, not the import.
  • The SAF direct-write assumes the picked destination's file descriptor is seekable (true for on-device storage like Downloads); exotic document providers are untested.

Housekeeping

src-tauri/gen/android/app/tauri.properties is now untracked. Tauri's own generated .gitignore already listed it, but a file committed before a rule exists stays tracked regardless — so it sat at versionCode=7003 while the app shipped 0.7.6, and versionCode is what Android compares to allow an update. Verified that tauri android build recreates it from tauri.conf.json, which is what CI runs.

Thanks for reading through all of it — and again, sorry for the size. Happy to break it up or adjust anything.

Pento and others added 3 commits July 11, 2026 21:40
Reduce Android memory pressure that led to OutOfMemoryError crashes
(RustWebViewClient.shouldInterceptRequest allocates each IPC/SQL payload
as one contiguous Java-heap block).

- Add EmbeddedImageMeta (EmbeddedImage without the heavy base64 imageData)
  and metadata-only DB reads (getEmbeddedImageMetaForStory,
  getEmbeddedImageIdsForStory, countStories). Hot paths that only need
  metadata (message send, retry backup, gallery list) no longer pull the
  base64 of every story image through the bridge in one buffer.
- Retry backup now stores embedded-image ids instead of full images; the
  ids are all the undo path (set-difference delete) ever needed.
- Gallery lazily loads each image's pixels via IntersectionObserver, so a
  story with many images never loads them all at once.
- Only buffer AI response bodies for the debug panel when debug logging is
  active, instead of cloning and reading every streamed response.
- Batch entity deletes on retry/undo (DELETE ... WHERE id IN (...), chunked
  under SQLite's 999-parameter limit) instead of one IPC round-trip per id.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013VbdCmGtp3WtyqAgirVMpi
Definitive fix for the Android OutOfMemoryError crashes: the heavy byte
payloads never cross the Tauri IPC bridge nor land in the WebView JS heap.
Only paths and ids travel over IPC; the bytes are streamed file-to-file
(or DB-to-file) inside native code.

New Rust commands (src-tauri/src/backup.rs):
- backup_database: streams the VACUUMed DB straight into the ZIP on disk.
- restore_database: extracts aventura.db from the archive directly onto the
  live DB file (native safety copy + WAL/SHM cleanup); ignores legacy
  stories/*.avt entries in older backups.
- export_images_zip: one ordered streaming query, decoding/writing a single
  PNG at a time (peak memory = one image).
- export_single_image: decodes one image natively to a PNG file.

Frontend:
- backupService/imageExport now invoke the native commands; the backup is
  DB-only (settings already live inside the DB, so settings.json was dropped).
- Remove the chunked-IPC helper (fileStream.ts) and the now-unused jszip
  dependency; drop the outdated .avt mention in the backup settings copy.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013VbdCmGtp3WtyqAgirVMpi
Route large exports, backups and restores straight to the user-chosen
SAF destination natively, fixing the Android OutOfMemory crashes caused
by streaming hundreds of MB through the JS/IPC bridge. Also: show
Pollinations image models (AventurasTeam#330), adjustable chapter summary length
(AventurasTeam#116), a generation info popover, and assorted memory/generation
improvements.
@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Caution

Review failed

An error occurred during the review process. Please try again later.

📝 Walkthrough

Walkthrough

The pull request adds native Rust backup, restore, and image export commands; switches frontend export flows to resolved save targets; reduces embedded-image transfer through metadata and IDs; adds configurable summary detail, paid Pollinations model filtering, generation metadata display, standardized errors, and stricter Android build setup.

Changes

Native backup and export

Layer / File(s) Summary
Native database and image operations
src-tauri/src/backup.rs, src-tauri/src/lib.rs, src-tauri/Cargo.toml
Database backups, restores, image exports, story exports, and Android SAF staging now run through registered Rust commands.
Frontend export integration
src/lib/services/backupService.ts, src/lib/services/export.ts, src/lib/services/imageExport.ts, src/lib/services/exportTarget.ts, src/lib/services/lorebookImportExport/..., src/lib/services/packs/import-export.ts
Exporters use a shared save-target resolver and native commands where applicable, while backups return the selected path.
Metadata-only image paths
src/lib/services/database.ts, src/lib/stores/ui.svelte.ts, src/lib/components/story/GalleryTab.svelte, src/lib/components/story/ActionInput.svelte
Gallery and retry flows carry image metadata or IDs, loading image bytes lazily when needed.
Retry cleanup batching
src/lib/stores/story.svelte.ts, src/lib/services/generation/RetryService.ts, src/lib/services/generation/phases/PreGenerationPhase.ts
Retry and entity cleanup no longer persist embedded image payloads and use batched database deletion methods.

Configurable generation and model selection

Layer / File(s) Summary
Summary detail configuration
src/lib/types/index.ts, src/lib/components/memory/*, src/lib/services/ai/*, src/lib/services/generation/ChapterService.ts, src/lib/services/prompts/templates/memory.ts
A concise/auto/precise setting flows from memory settings into chapter summarization prompts.
Paid Pollinations models
src/lib/components/settings/*, src/lib/services/ai/image/providers/*, src/lib/services/ai/sdk/providers/modelFetcher.ts
Profile settings control inclusion of paid-only Pollinations models, and caches distinguish paid from free listings.
Generation metadata and diagnostics
src/lib/components/story/StoryEntry.svelte, src/lib/components/story/ActionInput.svelte, src/lib/services/ai/sdk/providers/fetch.ts, src/lib/services/ai/image/providers/fetchAdapter.ts, src/lib/utils/error.ts
Narration metadata is recorded and displayed, debug response buffering is conditional, and thrown values receive consistent messages.

Build tooling

Layer / File(s) Summary
Android build discovery and APK output
compileApk.sh, src-tauri/tauri.conf.json, package.json
The build script discovers compatible SDK, NDK, and JDK installations, targets aarch64 Debug APKs, and removes the obsolete JSZip dependency.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant ExperimentalSettings
  participant BackupService
  participant Tauri
  participant NativeBackup
  User->>ExperimentalSettings: Start backup
  ExperimentalSettings->>BackupService: createFullBackup()
  BackupService->>Tauri: invoke backup_database
  Tauri->>NativeBackup: stream database and metadata into ZIP
  NativeBackup-->>BackupService: saved destination path
  BackupService-->>ExperimentalSettings: return saved path
  ExperimentalSettings-->>User: show backup completion
Loading

Poem

A rabbit zipped bytes with a native bright flair,
While images now hop only when needed there.
Summaries choose concise, precise, or in-between,
Paid models appear on a Pollinations screen.
APKs find their tools beneath moonlit skies—
“Thump!” says the bunny, as safer exports arise.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% 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 matches the main change set: native SAF-based backup/export/restore plus Pollinations and summary-length fixes.
Linked Issues check ✅ Passed The PR adds Pollinations paid-model inclusion controls and a configurable summary-detail setting, which address #330 and #116.
Out of Scope Changes check ✅ Passed The changes stay focused on backup/export/import, Pollinations model listing, summary detail, and related UX/error-handling updates.

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.

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request moves heavy payload operations (database backup/restore, image exports, and story exports) entirely to the native Rust side to prevent Android OutOfMemoryError (OOM) crashes. It also introduces lazy loading of image data in the gallery view, batched database deletions, a 'Summary Detail' configuration option for chapter summarization, and a utility to extract human-readable error messages. Feedback on these changes includes suggestions to write the restored database to a temporary file first and atomically rename it to prevent corruption, clean up old temporary restore files in the cache directory on Android, and add optional chaining with fallback values in formatStoryTime to avoid potential runtime crashes.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src-tauri/src/backup.rs Outdated
Comment thread src-tauri/src/backup.rs
Comment thread src/lib/components/story/StoryEntry.svelte

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

🧹 Nitpick comments (3)
src/lib/types/index.ts (1)

100-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Keep one source of truth for the memory configuration types.

src/lib/services/ai/index.ts also declares SummaryDetail and MemoryConfig. Import and re-export these definitions from $lib/types there to prevent the parallel contracts from diverging.

🤖 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 `@src/lib/types/index.ts` around lines 100 - 109, Update the AI service
module’s SummaryDetail and MemoryConfig declarations to import these types from
$lib/types and re-export them, removing the duplicate local definitions so
src/lib/types/index.ts remains the single source of truth.
src/lib/components/settings/ExperimentalSettings.svelte (1)

148-150: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Import open statically.

Since @tauri-apps/plugin-dialog is already statically imported at the top of the file (for ask), you can also import open there to avoid the redundant dynamic import.

♻️ Proposed refactor

Update the top-level import to include open:

  import { ask, open } from '`@tauri-apps/plugin-dialog`'

And then simplify this function:

-    // Pick the backup file. On Android this returns a SAF content:// URI; on desktop a real path.
-    const { open } = await import('`@tauri-apps/plugin-dialog`')
-    const selected = await open({
+    // Pick the backup file. On Android this returns a SAF content:// URI; on desktop a real path.
+    const selected = await open({
🤖 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 `@src/lib/components/settings/ExperimentalSettings.svelte` around lines 148 -
150, Update the top-level `@tauri-apps/plugin-dialog` import in
ExperimentalSettings.svelte to include open alongside ask, then remove the
redundant dynamic import inside the backup-file selection flow and use the
statically imported open directly.
src/lib/services/export.ts (1)

156-170: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Clean up duplicated comments.

There is a duplicated comment block explaining the native .avt build process.

♻️ Proposed fix to remove duplication
     if (!target) return false
 
-    // Build the .avt natively: Rust injects each image's base64 from SQLite and writes the JSON
-    // straight to the real destination path — no image bytes cross the JS/IPC bridge.
     // Build the .avt natively: Rust injects each image's base64 from SQLite and writes the JSON
     // straight to the chosen destination (a SAF content:// URI on Android) — no image bytes cross
     // the JS/IPC bridge.
     await invoke<string>('export_story_avt', {
🤖 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 `@src/lib/services/export.ts` around lines 156 - 170, In the export flow around
resolveSaveTarget and export_story_avt, remove the duplicated native .avt build
comment and retain a single accurate comment describing Rust’s image injection
and destination-path 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 `@src-tauri/src/backup.rs`:
- Around line 345-381: Ensure every restore attempt removes the staged
`.tmp-restore-*` file created by `import_saf_to_temp`, including when restore
fails. Add cleanup in the restore command’s finally-equivalent/error paths,
preserving the existing restore result and handling cleanup as best-effort.
- Around line 114-138: The restore flow around the archive extraction must stage
the database in a temporary file instead of truncating the live target via
File::create(&target). Ensure extraction completes and the staged database is
readable/validated, handle safety-copy failure as an actual restore error, close
active database handles before replacement, then atomically rename the validated
temporary database over target and clean up temporary and WAL/SHM files on
failure or success.

In `@src/lib/components/memory/MemoryView.svelte`:
- Around line 158-161: Update the resummarize argument construction in
MemoryView to safely access story.memoryConfig.summaryDetail with optional
chaining and an appropriate fallback when memoryConfig is null, while preserving
the existing value when configuration is present.

In `@src/lib/components/story/GalleryTab.svelte`:
- Around line 71-85: Update lazyImage and ensureImageData so failed image-data
loads remain retryable instead of leaving a permanent loading state. Make
ensureImageData expose whether the cache fill succeeded, and only call
observer.unobserve(node) after success; preserve observation or provide an
explicit retry state on failure. Apply the same handling to the related
image-loading paths around the referenced sections.
- Around line 27-32: Bound imageDataCache by clearing it whenever the active
story changes and evicting payloads for images that are no longer near the
visible or prefetched range. Update the related loading state as needed so stale
requests do not repopulate evicted entries, preserving lazy loading for
currently relevant images. Anchor the changes to imageDataCache,
loadingImageIds, and the story-change logic around the referenced gallery flow.

In `@src/lib/services/backupService.ts`:
- Around line 147-156: Update the restore flow around database.close() and
invoke('restore_database', { zipPath }) to handle native restore failures. Catch
errors from restore_database and recover to a usable state by reopening the
existing database connection when possible, otherwise exit or relaunch through
the established application recovery path; preserve successful restore behavior.
- Around line 59-85: The backup flow around database.vacuumInto must not fall
back to the live aventura.db file when snapshot creation fails. Remove the
livePath fallback and make the operation fail with the existing no-snapshot
error when VACUUM INTO fails or produces no snapshot; retain cleanupTemp
handling for successful temporary snapshots.

---

Nitpick comments:
In `@src/lib/components/settings/ExperimentalSettings.svelte`:
- Around line 148-150: Update the top-level `@tauri-apps/plugin-dialog` import in
ExperimentalSettings.svelte to include open alongside ask, then remove the
redundant dynamic import inside the backup-file selection flow and use the
statically imported open directly.

In `@src/lib/services/export.ts`:
- Around line 156-170: In the export flow around resolveSaveTarget and
export_story_avt, remove the duplicated native .avt build comment and retain a
single accurate comment describing Rust’s image injection and destination-path
behavior.

In `@src/lib/types/index.ts`:
- Around line 100-109: Update the AI service module’s SummaryDetail and
MemoryConfig declarations to import these types from $lib/types and re-export
them, removing the duplicate local definitions so src/lib/types/index.ts remains
the single source of truth.
🪄 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: CHILL

Plan: Pro Plus

Run ID: 516596c7-b003-43ec-8061-6a3257e9377f

📥 Commits

Reviewing files that changed from the base of the PR and between 4c2a748 and 6dba668.

⛔ Files ignored due to path filters (2)
  • package-lock.json is excluded by !**/package-lock.json
  • src-tauri/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (46)
  • compileApk.sh
  • package.json
  • src-tauri/Cargo.toml
  • src-tauri/src/backup.rs
  • src-tauri/src/lib.rs
  • src-tauri/tauri.conf.json
  • src/lib/components/layout/Header.svelte
  • src/lib/components/lorebook/LorebookExportModal.svelte
  • src/lib/components/memory/MemorySettings.svelte
  • src/lib/components/memory/MemoryView.svelte
  • src/lib/components/settings/ExperimentalSettings.svelte
  • src/lib/components/settings/ProfileForm.svelte
  • src/lib/components/settings/tabs/api-connection.svelte
  • src/lib/components/settings/tabs/images.svelte
  • src/lib/components/story/ActionInput.svelte
  • src/lib/components/story/GalleryTab.svelte
  • src/lib/components/story/StoryEntry.svelte
  • src/lib/components/vault/VaultExportModal.svelte
  • src/lib/components/vault/prompts/PromptPackList.svelte
  • src/lib/services/ai/generation/MemoryService.ts
  • src/lib/services/ai/image/providers/fetchAdapter.ts
  • src/lib/services/ai/image/providers/pollinations.ts
  • src/lib/services/ai/image/providers/registry.ts
  • src/lib/services/ai/image/providers/types.ts
  • src/lib/services/ai/index.ts
  • src/lib/services/ai/sdk/providers/fetch.ts
  • src/lib/services/ai/sdk/providers/modelFetcher.ts
  • src/lib/services/backupService.ts
  • src/lib/services/database.ts
  • src/lib/services/export.ts
  • src/lib/services/export/ExportCoordinationService.ts
  • src/lib/services/exportTarget.ts
  • src/lib/services/generation/ChapterService.ts
  • src/lib/services/generation/GenerationPipeline.ts
  • src/lib/services/generation/RetryService.ts
  • src/lib/services/generation/phases/PreGenerationPhase.ts
  • src/lib/services/imageExport.ts
  • src/lib/services/lorebookImportExport/export/vault.ts
  • src/lib/services/lorebookImportExport/export/write.ts
  • src/lib/services/packs/import-export.ts
  • src/lib/services/prompts/templates/memory.ts
  • src/lib/services/templates/variables.ts
  • src/lib/stores/story.svelte.ts
  • src/lib/stores/ui.svelte.ts
  • src/lib/types/index.ts
  • src/lib/utils/error.ts
💤 Files with no reviewable changes (2)
  • package.json
  • src/lib/services/generation/RetryService.ts

Comment thread src-tauri/src/backup.rs Outdated
Comment thread src-tauri/src/backup.rs
Comment thread src/lib/components/memory/MemoryView.svelte
Comment thread src/lib/components/story/GalleryTab.svelte
Comment thread src/lib/components/story/GalleryTab.svelte Outdated
Comment thread src/lib/services/backupService.ts
Comment thread src/lib/services/backupService.ts
@Pento95
Pento95 marked this pull request as draft July 15, 2026 22:37
@Pento95

Pento95 commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks both. I went through every finding and verified each one against the code rather than taking it at face value. 11 applied, 2 rejected as false positives. Details below.

Applied

backup.rs — restore truncated the live DB before extraction succeeded (gemini high / CodeRabbit critical)

Both correct, and this was the only genuinely serious one. File::create(&target) truncated aventura.db before io::copy ran, so a CRC error, a truncated archive or a full disk left the live database destroyed. The safety copy was worse than useless: its failure was only eprintln!'d, and nothing ever restored from it.

Fixed by staging the extracted DB next to the target, validating it, and only then swapping. One deviation from both suggestions: the safety copy is now a rename, not a std::fs::copy. It's atomic, instant, and costs no extra disk — the copy needed 3x the DB size (original + copy + staged), which on a ~270 MB Android DB is not a rounding error. If the second rename fails, the first is rolled back.

Also extracted the logic into restore_db_from_zip(zip_path, target) (plain paths, no AppHandle) so it's actually testable, and added 6 tests. I ran them against the old logic to confirm they catch the bug — the corrupt-archive case fails there with live DB was damaged: Invalid checksum, with the live DB overwritten by partial bytes from the new backup.

backup.rs — staged SAF temps never cleaned up (gemini medium / CodeRabbit major)

Real, and the doc comment claiming "the temp is best-effort cleaned up by the restore" was simply false — nothing removed them. These are full copies of the user's backup (~363 MB in my testing).

I went with @gemini-code-assist's approach over CodeRabbit's here: cleanup happens on the way in, at the top of import_saf_to_temp. A finally-equivalent in the restore command would never run in the normal case, because the app calls exit(0) on success — and it would still leak if the OS killed the app mid-restore. Sweeping on entry covers both.

backupService.ts — no live-DB fallback when VACUUM INTO fails (CodeRabbit major)

Valid, and I confirmed the premise empirically before acting: the header bytes of my aventura.db are 02 02, i.e. WAL is on. Archiving the bare .db really could ship a backup missing the newest commits — the worst failure mode for a backup, since it only surfaces at restore time. Fallback removed; a failed VACUUM INTO is now a hard error.

GalleryTab.svelte — unbounded base64 cache (CodeRabbit major)

Valid, and thematically the important one on an OOM PR. imageDataCache was cleared in refreshImages() but not on story change, and had no eviction — lazy loading bounds the peak at open, not the total. Now an LRU with a 96 MB budget (base64 length is a good proxy for the heap it costs), cleared on story change.

GalleryTab.svelte — failed images stuck on a permanent spinner (CodeRabbit minor)

Valid. observer.unobserve(node) ran synchronously right after the un-awaited ensureImageData(id), so it fired regardless of outcome, and the error was swallowed. Dropped the unobserve entirely — it was also wrong for the new LRU, since an evicted image must reload when scrolled back — and added a failedImageIds set rendering a Retry button in both grid and lightbox.

export.ts — duplicated comment (CodeRabbit nitpick) — real; the first copy was also stale about Android. Removed.

StoryEntry.svelteformatStoryTime optional chaining (gemini medium)

Applied, though as defensive hardening rather than a demonstrated bug: TimeTracker's fields are all required, and baseTime falls back to a complete default. The plausible path is a partial timeTracker from an imported .avt?? guards null, not incomplete objects. Cheap insurance, so in it goes.

ExperimentalSettings.svelte — static open import (CodeRabbit nitpick) — applied, cosmetic.

Rejected

MemoryView.svelte — "potential null pointer dereference on memoryConfig" (CodeRabbit major)

False positive. story.memoryConfig is a store getter, not a field:

get memoryConfig(): MemoryConfig {
  return this.currentStory?.memoryConfig || DEFAULT_MEMORY_CONFIG
}

It never returns null. The reasoning came from export.ts:392 (memoryConfig: data.story.memoryConfig || null), but that sets the field on the Story entity, which the getter then coalesces. story.memoryConfig.summaryDetail is already safe.

"Keep one source of truth for the memory configuration types" (CodeRabbit nitpick)

False positive. src/lib/services/ai/index.ts doesn't declare those types — it imports them from $lib/types (lines 49 and 57). A repo-wide grep finds exactly one declaration of each, in src/lib/types/index.ts. There is no parallel contract.

Notes

CodeRabbit's backupService.ts:147-156 ("app left with its DB connection closed") is diagnostically wrong — close() nulls the handle and getDb() re-opens via init(), as documented at database.ts:140-141. No reopen/relaunch needed. The real risk was reopening onto a DB that the failed restore had already truncated, which the staging fix resolves at the source.

Both bots reviewed 6dba668, which predates the removal of the Pollinations "Include paid models" toggle — CodeRabbit's release notes still advertise it. The API filters by tier server-side from the key, so the toggle was a client-side duplicate that was inert wherever the key was actually sent. The list now reflects what the key can really generate.

cargo test 6/6, svelte-check clean, cargo fmt clean.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Hi Pento95, thanks for the detailed breakdown of the fixes. It's great to see the thorough verification and the performance-conscious approach to the database restoration and image handling.

Regarding your notes:

  • Restore safety copy: Using an atomic rename (std::fs::rename) is indeed the correct approach for Android/Linux filesystems to ensure atomicity and avoid the overhead of copying large files. This is a solid improvement over a standard copy.
  • SAF cleanup: Your approach of cleaning up the temp file on entry (import_saf_to_temp) is robust, especially given the app lifecycle behavior you described. It's a pragmatic solution to avoid leaking large temporary files.
  • Gallery LRU cache: Implementing an LRU cache with a memory budget is the right way to handle image memory pressure. Using base64 length as a proxy for heap size is a clever and effective heuristic.

Your analysis of the "potential null pointer dereference" and the "source of truth" comments as false positives is correct based on the code structure you've described.

This PR looks very well-considered and addresses the critical OOM issues effectively. Nice work.

Addresses the Gemini/CodeRabbit review of AventurasTeam#367, plus bugs found while
verifying their findings.

Restore no longer destroys the database on failure. File::create truncated
the live aventura.db before extraction succeeded, so a CRC error or a full
disk left it in pieces with no rollback. The DB is now staged and validated
first, then swapped in by renaming the old file aside (atomic, and free in
disk space, which matters for a 270 MB DB on Android).

Import of a large .avt no longer fails. The Library's Import button read the
file with file.text(), which cannot exceed V8's max string length (~512 MiB);
a real 546 MB export failed with a misleading "not a valid JSON file". It now
goes through the native picker into a streaming Rust reader: structure stays
in TypeScript, only the payloads move. Peak memory is one image regardless of
file size (measured: 16 kB vs 108 MB for a naive parse of a 100 MB file).

Also fixed:
- Backups no longer fall back to the live DB file when VACUUM INTO fails. The
  DB runs in WAL mode, so that could ship a backup missing recent commits.
- Gallery images are now an LRU with a byte budget, cleared on story change.
  Lazy loading bounded the peak at open, not the total.
- Checkpoint snapshots kept stale chapter ids: checkpoints are imported before
  chapters, so remapping found nothing and silently kept the export's ids.
- Every export was stamped v1.7.0 while the importer checked for v1.8.0, so
  re-importing our own file always warned that the background image was lost.
- Android file pickers dropped their filters: the dialog plugin maps extensions
  to MIME types and silently discards unknown ones, including 'avt' and the '*'
  wildcard. Filters are now omitted there, where they only ever lied.
- Staged SAF restore temps (full copies of the backup) were never deleted.
- Removed the Pollinations "Include paid models" toggle: the API already
  filters by tier from the key, so it was a client-side duplicate that was
  inert wherever the key was actually sent.

Adds vitest and the first frontend tests (33), covering the import remapping
this change refactors, plus 14 Rust tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Pento95
Pento95 marked this pull request as ready for review July 16, 2026 23:00
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Caution

Review failed

An error occurred during the review process. Please try again later.


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.

1 similar comment
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Caution

Review failed

An error occurred during the review process. Please try again later.


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.

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.

Pollinations image list missing image models Adjustable chapter summary length

2 participants