fix(sqlite): repair fresh schema bootstrap - #1811
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds fresh-install SQLite schema metadata, switches session table creation to latest SQL, scopes diagnosis and repair APIs to an optional startup catalog, and runs advisory startup diagnosis with one guarded repair attempt in ChangesSQLite Fresh Schema Bootstrap
Sequence Diagram(s)sequenceDiagram
participant DatabaseInitializer
participant SQLitePresenter
participant SchemaInspector
participant DatabaseRepairService
DatabaseInitializer->>SQLitePresenter: construct presenter
DatabaseInitializer->>SQLitePresenter: diagnoseSchema(getStartupSchemaCatalog())
SQLitePresenter->>SchemaInspector: inspect startup catalog
SchemaInspector-->>SQLitePresenter: diagnosis
SQLitePresenter-->>DatabaseInitializer: diagnosis result
alt repairable issues and no prior repair
DatabaseInitializer->>SQLitePresenter: close()
DatabaseInitializer->>DatabaseRepairService: repair with startup catalog
DatabaseInitializer->>DatabaseInitializer: retry initialization
else manual issues or diagnosis failure
DatabaseInitializer->>DatabaseInitializer: log warnings and continue
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/main/presenter/lifecyclePresenter/DatabaseInitializer.ts`:
- Around line 66-75: The startup path in DatabaseInitializer should not claim
success when repairable schema issues still remain after a failed repair
attempt. Update the repair branch in DatabaseInitializer so that, when
repairAttempted is true and diagnosis.repairableIssues is still non-empty, it
does not log initialization success or return this.database; instead, continue
to fail startup or propagate the unresolved schema problem using the existing
diagnosis/formatSchemaIssues flow so the remaining missing table/column issues
are not masked.
In `@test/main/presenter/sqlitePresenter.test.ts`:
- Around line 159-163: The test is calling presenter.diagnoseSchema() without
the startup catalog and then filtering issues afterward, which can hide
regressions in startup-scoped diagnosis. Update the sqlitePresenter test to pass
getStartupSchemaCatalog!() directly into diagnoseSchema() and assert against
that scoped diagnosis result using the existing presenter.diagnoseSchema and
getStartupSchemaCatalog! symbols.
- Around line 202-206: The assertion in the sqlitePresenter bootstrap test is
too weak because it only checks that the recorded schema version is positive;
update the test to compare the stored value against the same latest schema
version source used by the production bootstrap logic. Use the existing schema
version helper or constant referenced by the presenter/bootstrap path, and
verify the single row returned from schema_versions matches that latest version
exactly.
🪄 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: dc3c06e9-9dd8-47d1-b1a7-9c5965c84ae1
📒 Files selected for processing (14)
docs/issues/sqlite-fresh-schema-bootstrap/plan.mddocs/issues/sqlite-fresh-schema-bootstrap/spec.mddocs/issues/sqlite-fresh-schema-bootstrap/tasks.mdsrc/main/presenter/lifecyclePresenter/DatabaseInitializer.tssrc/main/presenter/sqlitePresenter/index.tssrc/main/presenter/sqlitePresenter/schemaCatalog.tssrc/main/presenter/sqlitePresenter/schemaCatalogMetadata.tssrc/main/presenter/sqlitePresenter/schemaTypes.tssrc/main/presenter/sqlitePresenter/tables/deepchatSessions.tssrc/main/presenter/sqlitePresenter/tables/newSessions.tstest/main/presenter/lifecyclePresenter/DatabaseInitializer.test.tstest/main/presenter/sqlitePresenter.test.tstest/main/presenter/sqlitePresenter/schemaCatalogMetadata.test.tstest/main/presenter/sqlitePresenter/schemaRepair.test.ts
💤 Files with no reviewable changes (1)
- src/main/presenter/sqlitePresenter/tables/newSessions.ts
nexoracontrol-ops
left a comment
There was a problem hiding this comment.
Approved in CodeRabbit Change Stack
nexoracontrol-ops
left a comment
There was a problem hiding this comment.
Approved in CodeRabbit Change Stack
Fixes fresh
agent.dbbootstrap on new DeepChat installs.Previously, fresh startup could create
new_sessions/deepchat_sessionsfrom an empty recorded schema version, then stampschema_versionsto latest via the fresh fast-path. The database looked migrated but was missing latest columns such asnew_sessions.is_draft, causing session creation to fail withtable new_sessions has no column named is_draft.Summary by CodeRabbit