Repository navigation
docs: say what the better-sqlite3 adapter does about journal mode - #86
Conversation
The class comment said the adapter enables WAL, that readers do not block and that it recovers from corruption. open() runs no journal_mode pragma and the adapter has no recovery step, so a file made here runs the rollback journal, where a long read can make a commit fail busy. The comment and the README now say that. The changelog gets a section for this release and heads the entries under Unreleased with the versions that published them, as RELEASING.md asks.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis documentation-only PR corrects inaccurate better-sqlite3 claims about WAL, reader blocking, and corruption recovery, explains the SQLITE_BUSY risk when rollback-journal readers overlap commits, adds user guidance for enabling WAL, and updates changelog release headings. Sequence diagram for rollback-journal reader blocking a commitsequenceDiagram
participant Reader
participant SQLiteFile
participant Writer
Reader->>SQLiteFile: begin read transaction
Writer->>SQLiteFile: commit write
SQLiteFile-->>Writer: wait up to timeout
SQLiteFile-->>Writer: SQLITE_BUSY
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent |
Findings
|
| Layer / File(s) | Summary |
|---|---|
Document journal behavior src/adapters/betterSqliteAdapter.ts, README.md, CHANGELOG.md |
The documentation states that the adapter does not set a journal mode or recover from corruption. It explains that a long read can cause a commit to fail with SQLITE_BUSY after the 5,000 ms timeout, and advises setting WAL when reads and writes overlap. The changelog adds the 0.9.3 entry and removes the duplicated 0.9.2 entry. |
Estimated code review effort: 1 (Trivial) | ~4 minutes
Change: Other
Possibly related PRs
- framerslab/sql-storage-adapter#79: The release workflow uses a matching version section from
CHANGELOG.mdfor release notes, which connects to the new 0.9.3 changelog entry.
Merge Risk: 
Low · up to 7c424
-
Move the new changelog note under Unreleased before merging; the documentation changes otherwise present low risk.
Pre-merge checks: 
5
-
Full details: Title check
-
Explanation
The title "docs: say what the better-sqlite3 adapter does about journal mode" matches the main change. The adapter class comment is corrected to state that it sets no journal mode, and the README and CHANGELOG are updated to match. The title is specific and follows a conventional commit prefix. It does not mention the removal of the WAL and corruption-recovery claims, but it is not misleading.
Full details: Description check
-
Explanation
The description follows the repository template. It has a Summary that states what changed and why: the class comment wrongly claimed WAL was enabled automatically, and the adapter sets no journal mode. It has a Checklist; docs are updated and the title is a conventional commit. It has Related ("No issue"), Spec, plan and design ("None"), and Pictures ("No visible change"). The Tests section is extra. It explains why no tests were added, but the checklist item for tests is not checked. The description is complete and specific. The checklist boxes are not ticked, which is a minor gap.
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files. (2 skipped: 2 unsupported.)
-
Finishing Touches
-
Generate docstrings
-
- Commit to this branch
- Create a new PR
Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
PR Summary by QodoClarify better-sqlite3 journal mode and locking behavior
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
What changed
src/adapters/betterSqliteAdapter.ts: the class comment said the adapter "Automatically enables WAL mode". Itsopen()builds the database and runs onlyPRAGMA foreign_keyswhen asked, so it sets no journal mode. The comment now says so, under Limitations, and names the failure: a reader holding a read transaction can make a commit wait out better-sqlite3'stimeoutand failSQLITE_BUSY.README.md: one bullet besideforeignKeysin Configuration & Resolution, since the README has no better-sqlite3 section.CHANGELOG.md: a0.9.3section, and the entries underUnreleasedheaded with the versions that published them (0.9.2 for the build fix, 0.9.1 for the sql.js changes, from those releases' notes and theirpackage.json), so the new section reads in order.No code changes. Sources: journal modes, locking in rollback mode, WAL, better-sqlite3
timeout.Issue
No issue
Spec, plan and design
None
Pictures
No visible change
Docs
This pull request is the docs change: the class comment,
README.mdandCHANGELOG.md.Tests
A comment and docs have no end-to-end path. CI runs the usual suite on the branch, and the check is the release run on master: it publishes the next patch (
srcandREADME.mdchanged since 0.9.2), and the publisheddist/adapters/betterSqliteAdapter.d.tscarries the new comment.Summary by CodeRabbit