-
Notifications
You must be signed in to change notification settings - Fork 0
feat(db): Add Turso database integration with schema migrations #53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Configure Drizzle ORM with Turso connection - Create initial migration with location, locationLog, and locationLogImage tables - Update environment variables and dependencies - Add ESLint ignores for migration files
""" WalkthroughThis update introduces a Drizzle ORM-based SQLite database schema and integration. It adds environment validation for database credentials, migration files, schema definitions for three related tables, and updates development scripts and dependencies to support local database workflows. Minor adjustments are made to configuration and ignore files. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Env
participant DB (Drizzle ORM)
participant LibSQL
App->>Env: Load TURSO_DATABASE_URL & TURSO_AUTH_TOKEN
App->>DB: Initialize ORM with schema and credentials
DB->>LibSQL: Connect using URL and optional token
App->>DB: Perform queries (CRUD on location, locationLog, locationLogImage)
DB->>LibSQL: Execute SQL operations
LibSQL-->>DB: Return results
DB-->>App: Return data
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
🧹 Nitpick comments (5)
eslint.config.mjs (1)
18-18
: Remove duplicate ignore pattern.The
"**/migrations/*"
pattern appears twice in the ignores array, which is redundant.- ignores: [".pnpm-store/**", "**/migrations/*","*.config.***", "**/migrations/*"], + ignores: [".pnpm-store/**", "**/migrations/*", "*.config.***"],drizzle.config.ts (1)
2-2
: Relative import may break when Drizzle Kit runs the config
drizzle-kit
executes this file with its own TS loader; the working directory is the project root.
If./lib/env
itself imports project files that rely on paths/aliases not resolvable in that context, the CLI will crash. Consider requiring plainprocess.env
here or using a very small “config-only” module to avoid pulling the whole runtime env layer into the migration toolchain.package.json (1)
23-25
: Pin Turso CLI or document its requirement
"dev:db": "turso dev --db-file local.db"
assumes the binary is globally installed and on the PATH.
CI and fresh contributors will fail to runpnpm dev
. Either add it as an npm package (e.g.@turso/cli
) or document the requirement in the README.lib/db/migrations/0000_wonderful_betty_brant.sql (2)
24-24
: Consider cascading deletes for child rowsIf a
location
orlocationLog
is removed, dangling rows inlocationLog
/locationLogImage
remain.
Typical UX is to cascade:- FOREIGN KEY (`location_id`) REFERENCES `location`(`id`) ON UPDATE no action ON DELETE no action + FOREIGN KEY (`location_id`) REFERENCES `location`(`id`) ON DELETE CASCADESame for
locationLogImage.location_log_id
.Also applies to: 33-33
28-31
:key
column name is ambiguous & reserved in many SQL enginesEven though SQLite allows back-ticked identifiers, consider renaming to
storage_key
or similar for clarity and cross-DB safety.Also applies to: 185-191
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
.gitignore
(1 hunks)drizzle.config.ts
(1 hunks)eslint.config.mjs
(1 hunks)lib/db/index.ts
(1 hunks)lib/db/migrations/0000_wonderful_betty_brant.sql
(1 hunks)lib/db/migrations/meta/0000_snapshot.json
(1 hunks)lib/db/migrations/meta/_journal.json
(1 hunks)lib/db/schema/index.ts
(1 hunks)lib/db/schema/location-log-image.ts
(1 hunks)lib/db/schema/location-log.ts
(1 hunks)lib/db/schema/location.ts
(1 hunks)lib/env.ts
(1 hunks)package.json
(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
lib/db/schema/location-log-image.ts (1)
lib/db/schema/location-log.ts (1)
locationLog
(5-16)
lib/db/schema/location-log.ts (1)
lib/db/schema/location.ts (1)
location
(3-12)
🔇 Additional comments (7)
.gitignore (1)
30-32
: LGTM! Proper local database file exclusion.The addition of the
local.db*
pattern with descriptive comment correctly excludes local Turso database files from version control, supporting the new local development setup.lib/db/schema/index.ts (1)
1-3
: All referenced schema modules exist – approving exportsVerified that the following files are present under
lib/db/schema/
:
location.ts
location-log.ts
location-log-image.ts
No further action needed; the schema index exports are valid.
lib/db/index.ts (2)
7-14
: Good database configuration with conditional auth token.The database configuration correctly handles development vs production environments by conditionally applying the auth token. The snake_case naming convention and schema integration are well-implemented.
5-5
: Schema import verifiedThe schema index file (
lib/db/schema/index.ts
) exists and correctly re-exports the modules:
./location
./location-log
./location-log-image
No further action required.
drizzle.config.ts (1)
8-11
: No changes needed indrizzle.config.ts
—env vars are already validated by Zod
The importedenv
object comes fromlib/env.ts
, which uses a Zod schema andEnvSchema.parse(process.env)
to enforce thatTURSO_DATABASE_URL
andTURSO_AUTH_TOKEN
are present, yielding clear, descriptive errors if they’re missing. Adding fall-backs in the config would never be hit and isn’t necessary.If you do want to allow omitting
TURSO_AUTH_TOKEN
in development, update the schema inlib/env.ts
instead:- const EnvSchema = z.object({ - NODE_ENV: z.string(), - TURSO_DATABASE_URL: z.string(), - TURSO_AUTH_TOKEN: z.string(), - }); + const EnvSchema = z.object({ + NODE_ENV: z.string(), + TURSO_DATABASE_URL: z.string(), + TURSO_AUTH_TOKEN: z.string().optional(), + });Then keep your config as-is:
dbCredentials: { url: env.TURSO_DATABASE_URL, authToken: env.NODE_ENV === "development" ? undefined : env.TURSO_AUTH_TOKEN, },This preserves clear validation and only relaxes the token requirement in dev.
Likely an incorrect or invalid review comment.
lib/db/migrations/0000_wonderful_betty_brant.sql (1)
17-19
: Allow NULL forended_at
to support ongoing logsLogs in progress won’t have an end time yet. Mark
ended_at
as nullable, then add a CHECK enforcingended_at >= started_at
when it is set.- `ended_at` integer NOT NULL, + `ended_at` integer, + CHECK (ended_at IS NULL OR ended_at >= started_at),Also applies to: 112-118
lib/db/migrations/meta/0000_snapshot.json (1)
38-51
: Snapshot reflects the same latitude/longitude & timestamp issuesThe meta snapshot should be regenerated after applying the CHECK constraints, column renames, nullable
ended_at
, and cascading FKs to keep Drizzle in sync with the authoritative schema.Also applies to: 119-132, 192-198
TURSO_DATABASE_URL: z.string(), | ||
TURSO_AUTH_TOKEN: z.string(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix environment variable consistency issue.
TURSO_AUTH_TOKEN
is marked as required in the schema but is conditionally used in lib/db/index.ts
(set to undefined
in development). This inconsistency will cause validation failures in development environments.
Make TURSO_AUTH_TOKEN
optional to align with its conditional usage:
- TURSO_AUTH_TOKEN: z.string(),
+ TURSO_AUTH_TOKEN: z.string().optional(),
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
TURSO_DATABASE_URL: z.string(), | |
TURSO_AUTH_TOKEN: z.string(), | |
TURSO_DATABASE_URL: z.string(), | |
TURSO_AUTH_TOKEN: z.string().optional(), |
🤖 Prompt for AI Agents
In lib/env.ts around lines 7 to 8, the environment variable TURSO_AUTH_TOKEN is
currently required in the schema but is conditionally used and set to undefined
in development in lib/db/index.ts. To fix this inconsistency and prevent
validation failures, update the schema to make TURSO_AUTH_TOKEN optional by
changing its definition to allow undefined or making it optional in the zod
schema.
@@ -0,0 +1,13 @@ | |||
{ | |||
"version": "7", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify the version inconsistency in migration journal.
The journal shows version "7" but the entry has version "6". This inconsistency should be verified to ensure migration integrity.
Also applies to: 7-7
🏁 Script executed:
#!/bin/bash
# Description: Check if migration files match the journal entries
# Expected: Migration files should correspond to journal entries
echo "Checking migration files..."
fd "\.sql$" lib/db/migrations/
echo "Journal entries:"
cat lib/db/migrations/meta/_journal.json | jq '.entries[].tag'
Length of output: 283
🏁 Script executed:
#!/bin/bash
# Display the full migration journal metadata for inspection
echo "Full journal file:"
jq '.' lib/db/migrations/meta/_journal.json
Length of output: 302
Confirm migration journal version mismatch
The migration journal (lib/db/migrations/meta/_journal.json
) reports "version": "7"
, but its only entry uses "version": "6"
, yet there is just one SQL migration (0000_wonderful_betty_brant.sql
). This discrepancy will break future migrations. Please update or regenerate the journal so that:
- The top-level
"version"
matches the number you’ve actually applied (entries count + starting offset). - Each entry’s
"version"
aligns with its position in that sequence.
Path to fix:
- lib/db/migrations/meta/_journal.json
🤖 Prompt for AI Agents
In lib/db/migrations/meta/_journal.json at line 2, the top-level "version" is
set to "7" while the only entry has "version" "6", causing a mismatch that will
break migrations. Update the top-level "version" to match the actual number of
applied migrations (likely "1" if only one SQL migration exists) and adjust the
entry's "version" to align with its correct sequence position. Regenerate or
manually fix the journal file so all version numbers are consistent and
sequential.
// You can specify any property from the libsql connection options | ||
const db = drizzle({ | ||
connection: { | ||
url: env.TURSO_DATABASE_URL!, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove risky non-null assertion operator.
The non-null assertion operator (!
) is unnecessary and risky since the environment validation in lib/env.ts
already ensures TURSO_DATABASE_URL
is defined.
- url: env.TURSO_DATABASE_URL!,
+ url: env.TURSO_DATABASE_URL,
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
url: env.TURSO_DATABASE_URL!, | |
url: env.TURSO_DATABASE_URL, |
🤖 Prompt for AI Agents
In lib/db/index.ts at line 9, remove the non-null assertion operator (!) from
env.TURSO_DATABASE_URL since the environment validation in lib/env.ts guarantees
it is defined, making the assertion redundant and risky.
lat: real().notNull(), | ||
long: real().notNull(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Avoid the SQL keyword long
; prefer lng
or longitude
SQLite will allow it, but many tools (and TypeScript’s generated types) need quoting which is easy to forget. Rename the property and keep the physical column name if you must:
- long: real().notNull(),
+ lng: real().notNull().$name("long"),
Also consider adding an index on (lat, long)
for geospatial look-ups.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
lat: real().notNull(), | |
long: real().notNull(), | |
lat: real().notNull(), | |
lng: real().notNull().$name("long"), |
🤖 Prompt for AI Agents
In lib/db/schema/location.ts at lines 8-9, rename the property `long` to `lng`
or `longitude` to avoid conflicts with the SQL keyword `long`. Keep the physical
column name as `long` if necessary by specifying it explicitly in the schema
definition. Additionally, add an index on the `(lat, long)` columns to optimize
geospatial queries.
started_at: int().notNull(), | ||
ended_at: int().notNull(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add a CHECK to guarantee started_at ≤ ended_at
Nothing prevents inserting an obviously invalid range. Drizzle lets you attach custom checks:
ended_at: int().notNull(),
+}).$check({
+ name: "ck_locationLog_started_before_ended",
+ constraint: sql`started_at <= ended_at`,
});
This protects data integrity and simplifies querying for active logs.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
started_at: int().notNull(), | |
ended_at: int().notNull(), | |
started_at: int().notNull(), | |
ended_at: int().notNull(), | |
}).$check({ | |
name: "ck_locationLog_started_before_ended", | |
constraint: sql`started_at <= ended_at`, | |
}); |
🤖 Prompt for AI Agents
In lib/db/schema/location-log.ts around lines 9 to 10, add a CHECK constraint to
ensure that the started_at value is always less than or equal to the ended_at
value. This involves modifying the schema definition to include a custom check
condition that enforces started_at ≤ ended_at, thereby preventing invalid time
ranges from being inserted and maintaining data integrity.
key: text().notNull(), | ||
locationLogId: int().notNull().references(() => locationLog.id), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
key
is a reserved word; give it a safer column alias and add a UNIQUE index
- key: text().notNull(),
+ imageKey: text().notNull().unique().$name("key"),
This avoids quoting issues and enforces that the same image key isn’t inserted twice.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
key: text().notNull(), | |
locationLogId: int().notNull().references(() => locationLog.id), | |
imageKey: text().notNull().unique().$name("key"), | |
locationLogId: int().notNull().references(() => locationLog.id), |
🤖 Prompt for AI Agents
In lib/db/schema/location-log-image.ts around lines 7 to 8, rename the column
currently named "key" to a safer alias such as "imageKey" to avoid reserved word
conflicts. Update the schema definition accordingly and add a UNIQUE index on
this new column to ensure no duplicate image keys can be inserted.
"dev": "concurrently \"pnpm dev:db\" \"nuxt dev\"", | ||
"dev:db": "turso dev --db-file local.db", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
concurrently
is dev-only; move it out of production deps
Keeping it under dependencies
bloats the production bundle. Shift it to devDependencies
:
- "concurrently": "^9.2.0",
and in the block below:
+ "concurrently": "^9.2.0",
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
"dev": "concurrently \"pnpm dev:db\" \"nuxt dev\"", | |
"dev:db": "turso dev --db-file local.db", | |
"dependencies": { | |
- "concurrently": "^9.2.0", | |
// …other runtime dependencies | |
}, | |
"devDependencies": { | |
+ "concurrently": "^9.2.0", | |
// …other dev-only dependencies | |
}, | |
"scripts": { | |
"dev": "concurrently \"pnpm dev:db\" \"nuxt dev\"", | |
"dev:db": "turso dev --db-file local.db", | |
// …other scripts | |
} |
🤖 Prompt for AI Agents
In package.json around lines 8 to 9, the "concurrently" package is currently
listed under "dependencies" but it is only needed for development. Move
"concurrently" from the "dependencies" section to the "devDependencies" section
to avoid bloating the production bundle. Ensure the package is removed from
"dependencies" and added with the correct version under "devDependencies".
`created_at` integer NOT NULL, | ||
`updated_at` integer NOT NULL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Default timestamps are missing
created_at
/ updated_at
are NOT NULL
yet have no DEFAULT
.
You’ll have to set the values manually on every insert / update, which is error-prone.
- `created_at` integer NOT NULL,
- `updated_at` integer NOT NULL
+ `created_at` integer NOT NULL DEFAULT (strftime('%s','now')),
+ `updated_at` integer NOT NULL DEFAULT (strftime('%s','now'))
(or use DATETIME
with CURRENT_TIMESTAMP
).
Remember to add an ON UPDATE
trigger to keep updated_at
in sync.
Also applies to: 52-60, 140-148, 199-207
🤖 Prompt for AI Agents
In lib/db/migrations/0000_wonderful_betty_brant.sql around lines 8 to 9, the
created_at and updated_at columns are defined as NOT NULL integers but lack
default values, requiring manual timestamp management. Modify these columns to
use DATETIME type with DEFAULT CURRENT_TIMESTAMP for created_at, and DEFAULT
CURRENT_TIMESTAMP with ON UPDATE CURRENT_TIMESTAMP for updated_at to automate
timestamping. Apply similar changes to lines 52-60, 140-148, and 199-207 to
ensure consistency across all timestamp columns.
`lat` real NOT NULL, | ||
`long` real NOT NULL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add CHECK constraints & rename long
to avoid keyword clash
lat
/ long
are stored as plain real
without any domain enforcement.
A quick safeguard:
- `lat` real NOT NULL,
- `long` real NOT NULL,
+ `lat` real NOT NULL CHECK (lat BETWEEN -90 AND 90),
+ `lng` real NOT NULL CHECK (lng BETWEEN -180 AND 180),
long
is a reserved word in several SQL dialects and may break portability; lng
or longitude
is safer.
Same update is needed in the dependent tables & TypeScript schema.
Also applies to: 45-50
🤖 Prompt for AI Agents
In lib/db/migrations/0000_wonderful_betty_brant.sql around lines 6 to 7 and also
lines 45 to 50, replace the column name `long` with `lng` or `longitude` to
avoid using a reserved SQL keyword. Additionally, add CHECK constraints to the
`lat` and `lng` columns to enforce valid geographic coordinate ranges (e.g.,
latitude between -90 and 90, longitude between -180 and 180). Apply these
changes consistently in all dependent tables and update the corresponding
TypeScript schema to reflect the new column names and constraints.
Add required Turso database connection variables to CI workflow configuration to support the new database integration in build jobs. This ensures the Turso client can authenticate and connect to the database during automated testing and deployment pipelines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/lint.yml (1)
12-15
: Scope secrets to the single step that actually imports the code
pnpm install
andpnpm lint
are the only steps executed; onlypnpm lint
evaluates project source and triggers env-validation that demands the two Turso variables. Defining the secrets at the job level unnecessarily exposes them to every step (and any future ones) and widens the blast radius if a step ever prints its environment.Move the
env:
block under the- run: pnpm lint
step (or wrap that step in a separate job) so the secrets are available only where strictly needed.
closes #14
Summary by CodeRabbit
New Features
Chores
Documentation