feat(opencode): migrate from JSON files to SQLite database#879
feat(opencode): migrate from JSON files to SQLite database#879yukukotani wants to merge 2 commits intoryoppippi:mainfrom
Conversation
- Update data loading to read from opencode.db SQLite database instead of individual JSON files - Parse message data from message table and session metadata from session table - Update documentation to reflect SQLite storage location and structure - Bump Node.js minimum version from 20.19.4 to 22.13.0 - Remove unused dependencies: fast-sort, path-type, tinyglobby - Rewrite tests to use fs-fixture and database seeding - Support both Bun and Node.js runtime SQLite APIs
|
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 (1)
📝 WalkthroughWalkthroughThe PR migrates OpenCode storage from per-message/session JSON files to a central SQLite database (opencode.db). Data loading is refactored to query DB tables, message payloads now contain tokens/cost in JSON, schemas and parsing adapt to row-based shapes, Node engine target bumped to Node 22 and some devDeps removed. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as Loader
participant DB as SQLite(opencode.db)
participant Parser as RowParser/Validator
participant Consumer as LoadedUsage/Session
CLI->>DB: open readonly connection (bun:sqlite / node:sqlite)
CLI->>DB: execute SELECT messages/sessions
DB-->>CLI: return rows (id, session_id, time_created, data JSON...)
CLI->>Parser: parse row.data JSON & validate row schema
Parser-->>CLI: return domain object (message/session)
CLI->>Consumer: convert to LoadedUsageEntry / LoadedSessionMetadata
Consumer-->>CLI: consumed by downstream reporting / tests
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
yukukotani
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
| }, | ||
| "engines": { | ||
| "node": ">=20.19.4" | ||
| "node": ">=22.13.0" |
There was a problem hiding this comment.
Bumping this since node:sqlite was introduced in Node v22. We can stick with v20 if adding a dependency is okay.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/opencode/package.json (1)
47-64:⚠️ Potential issue | 🔴 CriticalMissing
path-typedependency breaks build.The
path-typepackage was removed fromdevDependencies, butdata-loader.ts(line 17) still importsisDirectorySyncfrom it:import { isDirectorySync } from 'path-type';This is used in
getOpenCodePath()(lines 187, 194) and will cause runtime failures.Either restore the dependency or replace the import with the newly added
isFile()helper pattern (usingstatSyncwrapped inResult.try).🐛 Option A: Restore dependency
"devDependencies": { "@ccusage/internal": "workspace:*", "@ccusage/terminal": "workspace:*", "@praha/byethrow": "catalog:runtime", + "path-type": "catalog:runtime",🐛 Option B: Replace with native check in data-loader.ts
Add a helper similar to
isFile():function isDirectory(targetPath: string): boolean { const statsResult = getPathStats(targetPath); if (Result.isFailure(statsResult)) { return false; } return statsResult.value.isDirectory(); }Then replace
isDirectorySynccalls withisDirectory.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/opencode/package.json` around lines 47 - 64, The build breaks because data-loader.ts still imports isDirectorySync while path-type was removed; either re-add "path-type" to devDependencies or update data-loader.ts to remove the import and use the native helper pattern: add an isDirectory function that calls getPathStats (the same helper used by isFile) and returns false on failure and stats.isDirectory() on success, then replace all uses of isDirectorySync in getOpenCodePath with this new isDirectory; ensure you remove the path-type import and use Result.isFailure/Result.try semantics consistent with existing getPathStats/isFile logic.
🧹 Nitpick comments (2)
apps/opencode/src/data-loader.ts (2)
343-358: Consider making synchronous functions non-async.Both
loadOpenCodeSessions()andloadOpenCodeMessages()are markedasyncbut contain noawaitexpressions - they only call the synchronousqueryOpenCodeDatabase(). This works but adds unnecessary Promise wrapping overhead.♻️ Optional simplification
If backward compatibility allows, these could return synchronously:
-export async function loadOpenCodeSessions(): Promise<Map<string, LoadedSessionMetadata>> { +export function loadOpenCodeSessions(): Map<string, LoadedSessionMetadata> {However, keeping them async maintains API consistency if callers expect Promises.
Also applies to: 364-383
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/opencode/src/data-loader.ts` around lines 343 - 358, Remove unnecessary async/Promise wrapping for functions that don't use await: change loadOpenCodeSessions (and likewise loadOpenCodeMessages) to be synchronous by removing the async keyword and updating their return types from Promise<Map<string, LoadedSessionMetadata>> (or Promise<...>) to Map<string, LoadedSessionMetadata> (and the corresponding return for messages). Keep the existing logic that calls the synchronous queryOpenCodeDatabase() and ensure any callers are updated to handle the synchronous return (or left as-is if they already await; adjust callers to stop awaiting if you make this change).
16-17: Move test-only import inside vitest block.
fs-fixtureis only used within theif (import.meta.vitest != null)block (lines 495, 640). Importing it at the module level includes it in the production bundle unnecessarily.♻️ Proposed fix
Move the import inside the test block:
-import { createFixture } from 'fs-fixture'; import { isDirectorySync } from 'path-type';Then inside the vitest block:
if (import.meta.vitest != null) { const { afterEach, describe, expect, it, vi } = import.meta.vitest; + const { createFixture } = await import('fs-fixture');Note: Per coding guidelines, dynamic
await import()should be avoided. Instead, consider a conditional require pattern or accept the import for tree-shaking at build time if tsdown handles dead-code elimination for the vitest guard.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/opencode/src/data-loader.ts` around lines 16 - 17, The top-level import "import { createFixture } from 'fs-fixture';" is test-only and should be moved inside the vitest guard to avoid including it in production bundles: remove the module-level import and instead require it inside the existing "if (import.meta.vitest != null) { ... }" block where createFixture is used (the blocks referencing createFixture), e.g. const { createFixture } = require('fs-fixture'); so only test code loads fs-fixture; leave isDirectorySync at module scope if still needed by runtime code. Ensure all references to createFixture inside the vitest block use the locally required binding.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/opencode/src/data-loader.ts`:
- Around line 522-523: Replace the outdated modelID string 'claude-sonnet-4-5'
with the current Sonnet model name 'claude-sonnet-4-20250514' in the three
test/data fixtures where modelID is set (the occurrences near the existing
'claude-opus-4-20250514' entry); search for modelID: 'claude-sonnet-4-5' in
data-loader.ts and change each to modelID: 'claude-sonnet-4-20250514' so Sonnet
entries match the current naming convention used for Opus.
---
Outside diff comments:
In `@apps/opencode/package.json`:
- Around line 47-64: The build breaks because data-loader.ts still imports
isDirectorySync while path-type was removed; either re-add "path-type" to
devDependencies or update data-loader.ts to remove the import and use the native
helper pattern: add an isDirectory function that calls getPathStats (the same
helper used by isFile) and returns false on failure and stats.isDirectory() on
success, then replace all uses of isDirectorySync in getOpenCodePath with this
new isDirectory; ensure you remove the path-type import and use
Result.isFailure/Result.try semantics consistent with existing
getPathStats/isFile logic.
---
Nitpick comments:
In `@apps/opencode/src/data-loader.ts`:
- Around line 343-358: Remove unnecessary async/Promise wrapping for functions
that don't use await: change loadOpenCodeSessions (and likewise
loadOpenCodeMessages) to be synchronous by removing the async keyword and
updating their return types from Promise<Map<string, LoadedSessionMetadata>> (or
Promise<...>) to Map<string, LoadedSessionMetadata> (and the corresponding
return for messages). Keep the existing logic that calls the synchronous
queryOpenCodeDatabase() and ensure any callers are updated to handle the
synchronous return (or left as-is if they already await; adjust callers to stop
awaiting if you make this change).
- Around line 16-17: The top-level import "import { createFixture } from
'fs-fixture';" is test-only and should be moved inside the vitest guard to avoid
including it in production bundles: remove the module-level import and instead
require it inside the existing "if (import.meta.vitest != null) { ... }" block
where createFixture is used (the blocks referencing createFixture), e.g. const {
createFixture } = require('fs-fixture'); so only test code loads fs-fixture;
leave isDirectorySync at module scope if still needed by runtime code. Ensure
all references to createFixture inside the vitest block use the locally required
binding.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 629c3e86-42b8-41cd-ae6a-e29dca717bcd
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
apps/opencode/CLAUDE.mdapps/opencode/README.mdapps/opencode/package.jsonapps/opencode/src/data-loader.tsapps/opencode/tsdown.config.ts
1 participant
Summary
OpenCode has migrated data store from JSON files to SQLite database in v1.2.0.
This PR follows that migration.
Note
Summary by CodeRabbit
New Features
Documentation
Chores