Refactor sqlite layer creation to use local variable in makeSqlitePersistenceLive#7
Refactor sqlite layer creation to use local variable in makeSqlitePersistenceLive#7
Conversation
📝 WalkthroughWalkthroughA new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/server/src/persistence/Layers/Sqlite.ts (1)
56-59: Consider extracting the layer for consistency.For consistency with the refactored
makeSqlitePersistenceLive, consider extracting themakeRuntimeSqliteLayercall into a named constant:const memoryLayer = makeRuntimeSqliteLayer({ filename: ":memory:" }); export const SqlitePersistenceMemory = Layer.provideMerge(setup, memoryLayer);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/persistence/Layers/Sqlite.ts` around lines 56 - 59, Extract the inline makeRuntimeSqliteLayer({ filename: ":memory:" }) call into a named constant (e.g., memoryLayer) and use that constant in the Layer.provideMerge call for SqlitePersistenceMemory so it matches the refactor style of makeSqlitePersistenceLive; update the export to call Layer.provideMerge(setup, memoryLayer) and keep the constant name clear and adjacent to the SqlitePersistenceMemory declaration for readability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/server/src/persistence/Layers/Sqlite.ts`:
- Around line 56-59: Extract the inline makeRuntimeSqliteLayer({ filename:
":memory:" }) call into a named constant (e.g., memoryLayer) and use that
constant in the Layer.provideMerge call for SqlitePersistenceMemory so it
matches the refactor style of makeSqlitePersistenceLive; update the export to
call Layer.provideMerge(setup, memoryLayer) and keep the constant name clear and
adjacent to the SqlitePersistenceMemory declaration for readability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b7783313-6f2f-430c-9d10-857eeec527d9
📒 Files selected for processing (1)
apps/server/src/persistence/Layers/Sqlite.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/server/src/persistence/Layers/Sqlite.ts`:
- Around line 48-56: The generator for Effect.fn("makeSqlitePersistenceLive") is
missing its closing brace and the Layer.unwrap export, leaving export const
SqlitePersistenceMemory syntactically inside that function; close the Effect.fn
block properly and return/unwrap the Layer (use Layer.unwrap or the same pattern
used for other persistence layers) so the live layer definition is complete
(ensure the function name makeSqlitePersistenceLive and the Layer.unwrap call
are added/closed) and then the top-level export const SqlitePersistenceMemory
can appear after the closed block.
🪄 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: 8d416cd8-40af-4055-ac00-4f043290c176
📒 Files selected for processing (1)
apps/server/src/persistence/Layers/Sqlite.ts
| return makeSqlitePersistenceLayer({ | ||
| filename: dbPath, | ||
| spanAttributes: { | ||
| "db.name": path.basename(dbPath), | ||
| "service.name": "t3-server", | ||
| }, | ||
| }); | ||
|
|
||
| export const SqlitePersistenceMemory = Layer.provideMerge( | ||
| setup, | ||
| makeRuntimeSqliteLayer({ filename: ":memory:" }), | ||
| ); | ||
| export const SqlitePersistenceMemory = makeSqlitePersistenceLayer({ filename: ":memory:" }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only verification: show the affected block and confirm missing closure before Line 56.
nl -ba apps/server/src/persistence/Layers/Sqlite.ts | sed -n '41,62p'Repository: Josh-wt/t3code
Length of output: 98
🏁 Script executed:
cat -n apps/server/src/persistence/Layers/Sqlite.ts | sed -n '40,65p'Repository: Josh-wt/t3code
Length of output: 870
Blocker: makeSqlitePersistenceLive is not closed, causing a parse error at Line 56.
The generator function for Effect.fn("makeSqlitePersistenceLive") is missing its closing brace and the Layer.unwrap configuration. export const SqlitePersistenceMemory at line 56 cannot be parsed as a top-level statement because it remains syntactically nested inside the unclosed function.
Proposed fix
return makeSqlitePersistenceLayer({
filename: dbPath,
spanAttributes: {
"db.name": path.basename(dbPath),
"service.name": "t3-server",
},
});
+}, Layer.unwrap);
export const SqlitePersistenceMemory = makeSqlitePersistenceLayer({ filename: ":memory:" });🧰 Tools
🪛 Biome (2.4.10)
[error] 56-56: Illegal use of an export declaration not at the top level
(parse)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/server/src/persistence/Layers/Sqlite.ts` around lines 48 - 56, The
generator for Effect.fn("makeSqlitePersistenceLive") is missing its closing
brace and the Layer.unwrap export, leaving export const SqlitePersistenceMemory
syntactically inside that function; close the Effect.fn block properly and
return/unwrap the Layer (use Layer.unwrap or the same pattern used for other
persistence layers) so the live layer definition is complete (ensure the
function name makeSqlitePersistenceLive and the Layer.unwrap call are
added/closed) and then the top-level export const SqlitePersistenceMemory can
appear after the closed block.
1 participant
Motivation
makeSqlitePersistenceLiveby naming the layer before merging it withsetup.Description
sqliteLayerconstant frommakeRuntimeSqliteLayer({...})and returnLayer.provideMerge(setup, sqliteLayer)instead of inlining the call.Testing
Codex Task
Summary by cubic
Refactors sqlite persistence by introducing a shared builder and documenting layer ordering to prevent regressions. No behavior change; improves clarity and reuse.
makeSqlitePersistenceLayer(config)to mergesetupwithmakeRuntimeSqliteLayer.makeSqlitePersistenceLiveandSqlitePersistenceMemoryto use the builder.setup); preserved PRAGMA setup and span attributes.Written for commit d90deec. Summary will update on new commits.
Summary by CodeRabbit