recreate database on migration failure

[mrshmllow]

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Improved cache initialization robustness with automatic recovery from common failure scenarios.
  • Enhanced diagnostics logging to better track cache operation status.

[coderabbitai]

Warning

Review limit reached

@mrshmllow, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 43 minutes and 22 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

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

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6d12d34a-ba12-472e-8dcf-472de2de387d

📥 Commits

Reviewing files that changed from the base of the PR and between bc8ea90 and 29d39a6.

📒 Files selected for processing (2)

• CHANGELOG.md
• crates/core/src/cache/mod.rs

📝 Walkthrough

Walkthrough

The PR enhances cache initialization's resilience by detecting specific migration failures and recovering via database reset. InspectionCache::new() now conditionally recovers from version-related migration errors by dropping the stale database and recreating it with fresh migrations, while treating other migration errors as fatal with logged failure.

Changes

Cache Migration Error Recovery

Layer / File(s)	Summary
Migration error imports and setup crates/core/src/cache/mod.rs	MigrateError type and info logging are imported to enable classification and reporting of migration failures during cache initialization.
Selective cache recovery with reset helper crates/core/src/cache/mod.rs	InspectionCache::new() inspects migration results and detects VersionMissing or VersionTooOld errors, then calls reset_migration() to drop the stale pool, delete the cache file, and recreate the database with fresh migrations. Other errors are logged and return None.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A cache that stumbles on migrations old,
Now rises from the dust with tales retold,
Reset and rebuild, the DB anew—
Version troubles melt like morning dew! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.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 accurately and concisely describes the main change: implementing database recreation logic when migrations fail.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch marshmallow/p-wuktkpzwzkqz

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

crates/core/src/cache/mod.rs (1)

71-86: ⚡ Quick win

Unreachable code and unnecessary complexity.

Lines 84-85 are unreachable: the ? on line 74 already early-returns None if reset_migration fails, and the _ => arm returns directly. After the match, recovered_pool is always Some(...).

♻️ Simplify the error handling

     let result = MIGRATOR.run(&pool).await;
 
     if let Err(ref err) = result {
-        let recovered_pool = match err {
+        match err {
             MigrateError::VersionMissing(_) | MigrateError::VersionTooOld(_, _) => {
-                Some(Self::reset_migration(pool, &cache_path).await?)
+                let pool = Self::reset_migration(pool, &cache_path).await?;
+                return Some(Self { pool });
             }
             _ => {
-                error!("failed to run cache migrations: {:?}", result);
+                error!("failed to run cache migrations: {err:?}");
                 return None;
             }
-        };
-        if let Some(pool) = recovered_pool {
-            return Some(Self { pool });
         }
-        error!("failed to run cache migrations: {:?}", result);
-        return None;
     }
 
     Some(Self { pool })

🤖 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 `@crates/core/src/cache/mod.rs` around lines 71 - 86, The error handling around
`result` is more complex than necessary and contains unreachable code; simplify
by removing `recovered_pool` and the trailing error/log, and handle the match
directly: match on `err` (the `MigrateError`), for
`MigrateError::VersionMissing(_) | MigrateError::VersionTooOld(_, _)` call
`Self::reset_migration(pool, &cache_path).await?` and if that returns a pool
immediately return `Some(Self { pool })`, otherwise in the `_` arm log the error
with `error!("failed to run cache migrations: {:?}", result);` and return
`None`; ensure no redundant logs or unreachable branches remain.

🤖 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 `@crates/core/src/cache/mod.rs`:
- Around line 97-102: The cleanup currently calls drop(pool) then immediately
tokio::fs::remove_file(cache_path), which can race on Windows because drop(pool)
doesn't await connection shutdown; change InspectionCache::reset_migration to
call pool.close().await (awaiting the sqlx Pool::close) before attempting
tokio::fs::remove_file(cache_path) so the pool's connections are closed
deterministically and the sqlite file is not locked when removed.

---

Nitpick comments:
In `@crates/core/src/cache/mod.rs`:
- Around line 71-86: The error handling around `result` is more complex than
necessary and contains unreachable code; simplify by removing `recovered_pool`
and the trailing error/log, and handle the match directly: match on `err` (the
`MigrateError`), for `MigrateError::VersionMissing(_) |
MigrateError::VersionTooOld(_, _)` call `Self::reset_migration(pool,
&cache_path).await?` and if that returns a pool immediately return `Some(Self {
pool })`, otherwise in the `_` arm log the error with `error!("failed to run
cache migrations: {:?}", result);` and return `None`; ensure no redundant logs
or unreachable branches remain.

🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: dff8727f-9655-4e73-b10e-dfed8323b1dd

📥 Commits

Reviewing files that changed from the base of the PR and between d2d152e and bc8ea90.

📒 Files selected for processing (1)

• crates/core/src/cache/mod.rs