Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (9)
📒 Files selected for processing (24)
📝 WalkthroughWalkthroughMigrates the project from Prisma to Drizzle ORM with SQLite: adds Drizzle config, schema, migrations, DB initialization with pragmas and migration/seed utilities, removes Prisma artifacts, adds temperature utilities and a SideSelector UI component, and updates documentation and package scripts. Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant DBInit as src/db/index.ts
participant SQLite as better-sqlite3
participant Drizzle as Drizzle ORM
participant Schema as src/db/schema.ts
App->>DBInit: import { db, sqlite }
DBInit->>DBInit: resolve DATABASE_URL or default path
DBInit->>SQLite: new Database(path)
SQLite-->>DBInit: sqlite instance
DBInit->>SQLite: PRAGMA (WAL, synchronous, cache, mmap, foreign_keys)
DBInit->>Schema: import schema definitions
Schema-->>DBInit: schema objects
DBInit->>Drizzle: drizzle(sqlite, { schema })
Drizzle-->>DBInit: db instance
DBInit-->>App: export db & sqlite
App->>DBInit: register SIGINT/SIGTERM handlers
Note right of DBInit: On signal -> sqlite.close() -> process.exit()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 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 |
6650a65 to
fc2002d
Compare
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (9)
src/components/SideSelector/SideSelector.tsx (2)
47-48:ensureFalways called with hardcoded'F'— both calls are no-ops;SideSelectorPropslacks aunitfield.
ensureF(temp, 'F')unconditionally returnstempunchanged (the conversion only fires for'C'). SinceSideTempcarries nounit, the component has no runtime information to pass a meaningful unit. Either:
- Add
unit: TempUnittoSideSelectorPropsand thread it through, or- Remove the
ensureFwrapper and useleftSide.currentTemp/rightSide.currentTempdirectly (if the contract is always Fahrenheit).♻️ Option A — expose unit in props
+import type { TempUnit } from 'src/lib/tempUtils' interface SideSelectorProps { temperatures: { leftSide: SideTemp rightSide: SideTemp } + unit?: TempUnit } -export const SideSelector = ({ temperatures }: SideSelectorProps) => { +export const SideSelector = ({ temperatures, unit = 'F' }: SideSelectorProps) => { ... - const currentF = ensureF(leftSide.currentTemp, 'F') + const currentF = ensureF(leftSide.currentTemp, unit)♻️ Option B — drop ensureF if always Fahrenheit
- const currentF = ensureF(leftSide.currentTemp, 'F') - const targetF = ensureF(leftSide.targetTemp, 'F') + const currentF = leftSide.currentTemp + const targetF = leftSide.targetTempAlso applies to: 81-82
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SideSelector/SideSelector.tsx` around lines 47 - 48, SideSelector is always calling ensureF with hardcoded 'F' and SideSelectorProps lacks a unit, so either surface the temp unit or stop calling ensureF; implement the first option: add unit: TempUnit to SideSelectorProps, accept it in the SideSelector component, and replace hardcoded 'F' calls to ensureF(...) with ensureF(..., unit) for all uses (e.g., ensureF(leftSide.currentTemp, unit), ensureF(leftSide.targetTemp, unit), ensureF(rightSide.currentTemp, unit), ensureF(rightSide.targetTemp, unit)); ensure the prop type import/definition for TempUnit is available and update any call sites that instantiate SideSelector to pass the unit.
46-63: Duplicate IIFE blocks — extract a shared helper to eliminate the repetition and fix cascading ESLint style errors.The two
((): JSX.Element => { ... })()blocks are nearly identical and also trigger multiple ESLint violations (@stylistic/multiline-ternary,@stylistic/jsx-one-expression-per-line, missing EOL). Extracting a smallSideInfosub-component removes all of these at once and makes the stable-icon inconsistency impossible to re-introduce.♻️ Proposed refactor
+type Trend = 'up' | 'down' | 'stable' + +const TrendIcon = ({ trend }: { trend: Trend }) => { + if (trend === 'up') return <TrendingUp size={14} className="text-amber-500" /> + if (trend === 'down') return <TrendingDown size={14} className="text-sky-500" /> + return <TrendingUp size={14} className="text-zinc-400" /> +} + +const SideInfo = ({ side }: { side: SideTemp }) => { + const currentF = side.currentTemp // or ensureF(side.currentTemp, unit) + const targetF = side.targetTemp + const trend = determineTrend(currentF, targetF) + const scale = mapToEightSleepScale(currentF) + return ( + <div className="flex items-center gap-1.5 text-[13px]"> + <TrendIcon trend={trend} /> + <span> + {formatTemp(currentF, 'F')} + {' · '} + {scale} + /10 + </span> + </div> + ) +} // Inside SideSelector — replace both IIFE divs: - <div className="flex items-center gap-1.5 text-[13px]"> - { - ((): JSX.Element => { - ... - })() - } - </div> + <SideInfo side={leftSide} />Also applies to: 80-97
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SideSelector/SideSelector.tsx` around lines 46 - 63, The two IIFE JSX blocks in SideSelector that compute currentF/targetF and render the trend icon and temp (using ensureF, determineTrend, mapToEightSleepScale, and formatTemp) should be extracted into a small memoized subcomponent (e.g., SideInfo) within the same file; implement SideInfo to accept a side object (or currentTemp/targetTemp), compute currentF/targetF, trend, scale and return the consistent JSX using TrendingUp/TrendingDown and the stable-icon branch (no IIFE/ternary line-wrapping), then replace both IIFE usages with <SideInfo side={leftSide}/> and <SideInfo side={rightSide}/> to remove duplication and resolve the multiline-ternary/jsx-one-expression-per-line/EOL ESLint errors.src/db/index.ts (1)
1-21: Module-level side effects make testing and reuse harder.Importing this module immediately opens a DB connection, runs PRAGMAs, and registers signal handlers. This makes it difficult to:
- Use a different DB path in tests (e.g.,
:memory:).- Control connection lifecycle explicitly.
- Avoid side effects in modules that only need the types/schema.
Consider a lazy-initialization pattern (e.g.,
getDb()singleton) that defers connection creation to first use, or at least export a factory function alongside the eager exports for testability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/index.ts` around lines 1 - 21, The module currently opens a connection and applies PRAGMAs at import time (using DB_PATH, sqlite, and db), causing module-level side effects; change it to a lazy/factory pattern by moving Database creation, sqlite PRAGMA calls, and drizzle() initialization into an exported init/get function (e.g., initDb or getDb) that accepts an optional dbPath (so tests can pass ':memory:'), return the initialized db and sqlite handles, and also export a closeDb function to explicitly close the connection; ensure existing eager exports (sqlite, db) are removed or replaced with getters that call the initializer so importing the module no longer opens connections..claude/commands/pr-describe.md (1)
25-41: Fenced code block missing language identifier.The static analysis tool (markdownlint MD040) flags this code block. Adding
markdownas the language identifier would satisfy the linter and enable syntax highlighting.-``` +```markdown ## Summary🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/commands/pr-describe.md around lines 25 - 41, The fenced code block starting with "## Summary" needs a language identifier to satisfy markdownlint MD040; update the opening fence from ``` to ```markdown so the block reads ```markdown and retains all existing contents (e.g., "## Summary", "## Scope Breakdown", "## Test plan") to enable syntax highlighting and silence the linter.package.json (1)
7-11: No script entry for the programmatic migration runner with seeding.
db:migraterunsdrizzle-kit migrate, which is the CLI-based migrator and does not invokesrc/db/migrate.ts(the programmatic runner that also seeds default data). Consider adding a dedicated script so developers and CI can run migrations + seeding in one step:"db:migrate": "drizzle-kit migrate", + "db:setup": "tsx src/db/migrate.ts",This also avoids confusion between the two distinct migration paths.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 7 - 11, The package.json currently has "db:migrate" mapped to the CLI (drizzle-kit) which does not run your programmatic runner that seeds defaults; add a new npm script (e.g., "db:migrate:run" or "db:migrate:programmatic") that invokes your programmatic migration entry (src/db/migrate.ts) so migrations and seeding run together; update CI/dev docs to call this new script instead of the CLI one and ensure the script runs the compiled JS or uses the appropriate ts-node/runner for src/db/migrate.ts.src/db/migrate.ts (1)
17-18: ESLint@stylistic/brace-styleviolations on} catchblocks.The project's ESLint config requires the closing brace and
catchon separate lines (Allman-style or similar). Adjust to match:- } catch (error) { + } + catch (error) {Apply at both lines 17 and 67.
Also applies to: 67-68
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/migrate.ts` around lines 17 - 18, Two catch blocks in src/db/migrate.ts currently violate the project's brace-style by having the closing brace and "catch" on the same line; update both occurrences so the closing brace of the try block is on its own line and the "catch (error) { ... }" starts on the following line (i.e., move the catch keyword to a new line after the prior closing brace) for the try/catch blocks handling migration errors.src/db/schema.ts (3)
42-66:tapGestureshas no constraint linking optional fields toactionType.When
actionType = 'temperature', the columnstemperatureChangeandtemperatureAmountshould be non-null, andalarmBehavior/alarmSnoozeDuration/alarmInactiveBehaviorshould be null — and vice versa. Without acheck()constraint, all combinations (e.g., a temperature action withtemperatureChange = null) are silently valid at the DB level.🛡️ Optional check constraint using Drizzle's `check()`
-}, (t) => [ +}, (t) => [ + check( + 'tap_gesture_action_fields', + sql`(${t.actionType} = 'temperature' AND ${t.temperatureChange} IS NOT NULL AND ${t.temperatureAmount} IS NOT NULL) + OR + (${t.actionType} = 'alarm' AND ${t.alarmBehavior} IS NOT NULL)` + ), +])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/schema.ts` around lines 42 - 66, Add a DB-level CHECK constraint to tapGestures that enforces column nullability based on actionType: when actionType = 'temperature' require temperatureChange and temperatureAmount to be NOT NULL and ensure alarmBehavior, alarmSnoozeDuration, alarmInactiveBehavior are NULL; conversely when actionType = 'alarm' require alarmBehavior (and alarmSnoozeDuration if snooze) and/or alarmInactiveBehavior to be NOT NULL and ensure temperatureChange and temperatureAmount are NULL. Implement this by calling check(...) on the tapGestures table definition and express the two boolean clauses combined with OR (one clause for actionType='temperature' and one for actionType='alarm') referencing the existing column identifiers temperatureChange, temperatureAmount, alarmBehavior, alarmSnoozeDuration, alarmInactiveBehavior, and actionType.
11-13:{ enum: [...] }is TypeScript-only — no DB-level constraint enforcement.Drizzle's enum config for SQLite "won't check runtime values." Any direct SQL insert (raw migration, external tool, test fixture) can write values outside the defined enum without error. For safety-critical fields like
temperatureUnit,side,actionType, andtapType, consider pairing these with acheck()constraint:-import { integer, real, sqliteTable, text } from 'drizzle-orm/sqlite-core' +import { check, index, integer, real, sqliteTable, text } from 'drizzle-orm/sqlite-core'Example for
deviceSettings.temperatureUnit:- temperatureUnit: text('temperature_unit', { enum: ['F', 'C'] }) - .notNull() - .default('F'), + temperatureUnit: text('temperature_unit', { enum: ['F', 'C'] }) + .notNull() + .default('F'),-export const deviceSettings = sqliteTable('device_settings', { +export const deviceSettings = sqliteTable('device_settings', { // ...columns -}) +}, (t) => [ + check('temperature_unit_check', sql`${t.temperatureUnit} IN ('F', 'C')`), +])Drizzle supports
check()constraints via the table's third argument.Also applies to: 44-48
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/schema.ts` around lines 11 - 13, The enum options in the column definitions (e.g., temperatureUnit, side, actionType, tapType) are TypeScript-only and don’t enforce DB-level constraints; update the table definitions in src/db/schema.ts (e.g., the deviceSettings table and other tables that declare those enum columns) to add SQL CHECK constraints via the table’s third-argument options so the DB rejects invalid values at runtime—add a check() that asserts the column is one of the allowed literals (e.g., temperature_unit IN ('F','C')) for each affected column to mirror the TypeScript enum restrictions.
25-27:updatedAtis never auto-updated on row modifications.
.default(sql\(unixepoch())`)is a SQL-levelDEFAULT, so it is set only onINSERT. No trigger or ORM hook updates it onUPDATE. This applies to every table that has anupdatedAtcolumn (lines 25-27, 37-39, 63-65, 92-94, 118-120, 148-150). Application code must always explicitly setupdatedAt: new Date()` on every update call — if that discipline isn't enforced, the column will silently reflect stale creation timestamps.Either document this expectation clearly in a shared query helper, or add a SQLite trigger per table:
CREATE TRIGGER set_device_settings_updated_at AFTER UPDATE ON device_settings FOR EACH ROW BEGIN UPDATE device_settings SET updated_at = unixepoch() WHERE id = OLD.id; END;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/schema.ts` around lines 25 - 27, The updatedAt column defined as updatedAt: integer('updated_at', { mode: 'timestamp' }).default(sql`(unixepoch())`) only sets on INSERT and is never auto-updated on UPDATE; either add a per-table SQLite AFTER UPDATE trigger (e.g. CREATE TRIGGER set_<table>_updated_at AFTER UPDATE ON <table> FOR EACH ROW BEGIN UPDATE <table> SET updated_at = unixepoch() WHERE id = OLD.id; END;) for each table that defines updatedAt (device_settings and the other tables shown), or enforce updates in application-level update helpers by always setting updatedAt = Math.floor(Date.now()/1000) or new Date() (depending on your storage format) inside the shared update function so every update call updates the timestamp.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
generated/prisma/browser.tsis excluded by!**/generated/**generated/prisma/client.tsis excluded by!**/generated/**generated/prisma/commonInputTypes.tsis excluded by!**/generated/**generated/prisma/enums.tsis excluded by!**/generated/**generated/prisma/internal/class.tsis excluded by!**/generated/**generated/prisma/internal/prismaNamespace.tsis excluded by!**/generated/**generated/prisma/internal/prismaNamespaceBrowser.tsis excluded by!**/generated/**generated/prisma/models.tsis excluded by!**/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
.claude/commands/pr-describe.mddocs/adr/0010-drizzle-orm-sqlite.mddocs/adr/0010-prisma-sqlite.mddrizzle.config.tspackage.jsonprisma.config.tsprisma/schema.prismasrc/components/SideSelector/SideSelector.tsxsrc/db/index.tssrc/db/migrate.tssrc/db/migrations/0000_plain_silverclaw.sqlsrc/db/migrations/meta/0000_snapshot.jsonsrc/db/migrations/meta/_journal.jsonsrc/db/schema.tssrc/lib/prisma.tssrc/lib/tempUtils.ts
💤 Files with no reviewable changes (4)
- docs/adr/0010-prisma-sqlite.md
- src/lib/prisma.ts
- prisma.config.ts
- prisma/schema.prisma
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/adr/0010-drizzle-orm-sqlite.md`:
- Around line 87-88: The ADR currently sets sqlite.pragma('mmap_size =
30000000000') without documenting the 32-bit limitation; update the ADR text
near the sqlite.pragma examples (the lines with sqlite.pragma('temp_store =
MEMORY') and sqlite.pragma('mmap_size = 30000000000')) to explicitly state the
assumption that hosts are 64-bit and that mmap_size reserves virtual address
space (not physical RAM), and add a short warning recommending a much smaller
mmap_size or omitting the pragma on 32-bit embedded systems and explaining that
reads/writes will be used for ranges beyond mmap_size.
In `@src/components/SideSelector/SideSelector.tsx`:
- Line 58: The stable state icon is inconsistent: the left stable branch renders
<TrendingUp> while the right renders <TrendingDown>; update the SideSelector
stable rendering so both branches use the same neutral icon (choose either
<TrendingUp> with className "text-zinc-400" for consistency or replace both with
a neutral <Minus> icon from lucide-react) — locate the two IIFE/conditional
branches that render the stable state (the JSX where <TrendingUp ... /> and
<TrendingDown ... /> appear) and swap the icon so the same component and
className are used in both places.
- Around line 23-25: Remove the unused React state declarations in the
SideSelector component: delete the useState calls that define
temperature/setTemperature and activeTab/setActiveTab since they are never
referenced; remove any related imports or types only if they become unused as a
result (leave Side and activeSide intact). Target the lines where temperature,
setTemperature, activeTab, and setActiveTab are declared inside the SideSelector
function and remove them to resolve the ESLint no-unused-vars errors.
- Around line 10-19: The interfaces SideTemp and SideSelectorProps use
semicolons as member delimiters and there is an extra semicolon elsewhere;
update the interface member delimiters to match project ESLint
(`@stylistic/member-delimiter-style`) by replacing semicolons with commas (or
removing delimiters if the style expects none) for properties inside SideTemp
(currentTemp, targetTemp) and SideSelectorProps (temperatures.leftSide,
temperatures.rightSide), and remove the stray semicolon referenced in the diff
to satisfy `@stylistic/semi`.
In `@src/db/index.ts`:
- Around line 16-17: The PRAGMA usage in src/db/index.ts is problematic: reduce
the mmap_size PRAGMA used on the sqlite instance from 30000000000 to a
reasonable embedded-device-friendly value (e.g., 268435456) by updating the
sqlite.pragma('mmap_size = ...') call; and remove the sqlite.pragma('page_size =
4096') call from this runtime path (or alternatively move it into your one-time
DB-initialization flow that runs before any tables are created and follow it
with a VACUUM) so you don't silently attempt a no-op after migrations have run.
- Around line 24-34: Module-level registration of process.on('SIGINT'/'SIGTERM')
causes duplicate listeners; move handler registration out of import-time side
effects by creating and exporting a registerShutdownHandlers (or initDb)
function that attaches the handlers only once using a module-scoped boolean flag
(e.g. handlersRegistered) and inside the handlers call sqlite.close() then exit;
replace the current direct process.on(...) calls with this guarded function and
invoke it from your app bootstrap entrypoint instead of on import.
In `@src/db/migrate.ts`:
- Around line 26-71: seedDefaultData currently performs three separate inserts
(db.insert(deviceSettings), db.insert(sideSettings), db.insert(deviceState))
outside a transaction which can leave the DB partially seeded on failure; change
seedDefaultData to run the seeding inserts inside a single transactional block
(use your DB client's transaction API) so that all inserts execute atomically
and are rolled back on error, e.g., after checking existingSettings, open a
transaction and perform the three inserts (deviceSettings, sideSettings,
deviceState) inside it and rethrow on failure so the transaction will rollback.
- Around line 12-14: The migrationsFolder path is currently relative to
process.cwd() and should be resolved relative to this module; update the migrate
call so migrationsFolder is computed from the module location (use __dirname or
import.meta.url depending on module system) instead of the literal
'./src/db/migrations' so migrate(...) always points to the correct folder;
change the value passed to migrationsFolder in the migrate(...) invocation
accordingly.
In `@src/db/migrations/0000_plain_silverclaw.sql`:
- Around line 37-42: The migration is missing indexes for time-series and
schedule tables (vitals, movement, sleep_records, temperature_schedules,
power_schedules, alarm_schedules); either convert the raw SQL index definitions
in schema.ts to Drizzle's index API on the respective table definitions (e.g.,
add index(...) entries in the table factory callbacks for vitals, movement,
sleep_records, temperature_schedules, power_schedules, alarm_schedules) so
drizzle-kit will emit them, or add the corresponding CREATE INDEX statements
manually into this migration and then regenerate/apply the migration so the
indexes are present.
In `@src/db/schema.ts`:
- Around line 30-33: The sideSettings table sets name to a static default
'Left', so inserting a 'right' side without a name stores 'Left'; remove or
change this misleading default and ensure name is explicitly provided during
seeding/inserts (or add a DB CHECK/constraint) — update the sqliteTable
definition for sideSettings (the name column) to not default to 'Left' and
modify the seeding/insert logic that writes to sideSettings (where side is
'left'/'right') to supply the correct name based on the side value, or
alternatively add a constraint that enforces name matches side if that fits your
domain.
- Around line 229-253: The exported standalone index objects (vitalsIndexes,
movementIndexes, sleepRecordsIndexes, temperatureSchedulesIndexes,
powerSchedulesIndexes, alarmSchedulesIndexes) are ignored by Drizzle and thus
not created; move each raw sql index into its corresponding table definition by
importing and using the index() builder from "drizzle-orm/sqlite-core" and pass
the index(...) entries inside the third argument of the table declarations
(e.g., vitals, movement, sleepRecords, temperature_schedules, power_schedules,
alarm_schedules), then delete the standalone *Indexes exports so the Drizzle
migrator will generate CREATE INDEX statements.
---
Nitpick comments:
In @.claude/commands/pr-describe.md:
- Around line 25-41: The fenced code block starting with "## Summary" needs a
language identifier to satisfy markdownlint MD040; update the opening fence from
``` to ```markdown so the block reads ```markdown and retains all existing
contents (e.g., "## Summary", "## Scope Breakdown", "## Test plan") to enable
syntax highlighting and silence the linter.
In `@package.json`:
- Around line 7-11: The package.json currently has "db:migrate" mapped to the
CLI (drizzle-kit) which does not run your programmatic runner that seeds
defaults; add a new npm script (e.g., "db:migrate:run" or
"db:migrate:programmatic") that invokes your programmatic migration entry
(src/db/migrate.ts) so migrations and seeding run together; update CI/dev docs
to call this new script instead of the CLI one and ensure the script runs the
compiled JS or uses the appropriate ts-node/runner for src/db/migrate.ts.
In `@src/components/SideSelector/SideSelector.tsx`:
- Around line 47-48: SideSelector is always calling ensureF with hardcoded 'F'
and SideSelectorProps lacks a unit, so either surface the temp unit or stop
calling ensureF; implement the first option: add unit: TempUnit to
SideSelectorProps, accept it in the SideSelector component, and replace
hardcoded 'F' calls to ensureF(...) with ensureF(..., unit) for all uses (e.g.,
ensureF(leftSide.currentTemp, unit), ensureF(leftSide.targetTemp, unit),
ensureF(rightSide.currentTemp, unit), ensureF(rightSide.targetTemp, unit));
ensure the prop type import/definition for TempUnit is available and update any
call sites that instantiate SideSelector to pass the unit.
- Around line 46-63: The two IIFE JSX blocks in SideSelector that compute
currentF/targetF and render the trend icon and temp (using ensureF,
determineTrend, mapToEightSleepScale, and formatTemp) should be extracted into a
small memoized subcomponent (e.g., SideInfo) within the same file; implement
SideInfo to accept a side object (or currentTemp/targetTemp), compute
currentF/targetF, trend, scale and return the consistent JSX using
TrendingUp/TrendingDown and the stable-icon branch (no IIFE/ternary
line-wrapping), then replace both IIFE usages with <SideInfo side={leftSide}/>
and <SideInfo side={rightSide}/> to remove duplication and resolve the
multiline-ternary/jsx-one-expression-per-line/EOL ESLint errors.
In `@src/db/index.ts`:
- Around line 1-21: The module currently opens a connection and applies PRAGMAs
at import time (using DB_PATH, sqlite, and db), causing module-level side
effects; change it to a lazy/factory pattern by moving Database creation, sqlite
PRAGMA calls, and drizzle() initialization into an exported init/get function
(e.g., initDb or getDb) that accepts an optional dbPath (so tests can pass
':memory:'), return the initialized db and sqlite handles, and also export a
closeDb function to explicitly close the connection; ensure existing eager
exports (sqlite, db) are removed or replaced with getters that call the
initializer so importing the module no longer opens connections.
In `@src/db/migrate.ts`:
- Around line 17-18: Two catch blocks in src/db/migrate.ts currently violate the
project's brace-style by having the closing brace and "catch" on the same line;
update both occurrences so the closing brace of the try block is on its own line
and the "catch (error) { ... }" starts on the following line (i.e., move the
catch keyword to a new line after the prior closing brace) for the try/catch
blocks handling migration errors.
In `@src/db/schema.ts`:
- Around line 42-66: Add a DB-level CHECK constraint to tapGestures that
enforces column nullability based on actionType: when actionType = 'temperature'
require temperatureChange and temperatureAmount to be NOT NULL and ensure
alarmBehavior, alarmSnoozeDuration, alarmInactiveBehavior are NULL; conversely
when actionType = 'alarm' require alarmBehavior (and alarmSnoozeDuration if
snooze) and/or alarmInactiveBehavior to be NOT NULL and ensure temperatureChange
and temperatureAmount are NULL. Implement this by calling check(...) on the
tapGestures table definition and express the two boolean clauses combined with
OR (one clause for actionType='temperature' and one for actionType='alarm')
referencing the existing column identifiers temperatureChange,
temperatureAmount, alarmBehavior, alarmSnoozeDuration, alarmInactiveBehavior,
and actionType.
- Around line 11-13: The enum options in the column definitions (e.g.,
temperatureUnit, side, actionType, tapType) are TypeScript-only and don’t
enforce DB-level constraints; update the table definitions in src/db/schema.ts
(e.g., the deviceSettings table and other tables that declare those enum
columns) to add SQL CHECK constraints via the table’s third-argument options so
the DB rejects invalid values at runtime—add a check() that asserts the column
is one of the allowed literals (e.g., temperature_unit IN ('F','C')) for each
affected column to mirror the TypeScript enum restrictions.
- Around line 25-27: The updatedAt column defined as updatedAt:
integer('updated_at', { mode: 'timestamp' }).default(sql`(unixepoch())`) only
sets on INSERT and is never auto-updated on UPDATE; either add a per-table
SQLite AFTER UPDATE trigger (e.g. CREATE TRIGGER set_<table>_updated_at AFTER
UPDATE ON <table> FOR EACH ROW BEGIN UPDATE <table> SET updated_at = unixepoch()
WHERE id = OLD.id; END;) for each table that defines updatedAt (device_settings
and the other tables shown), or enforce updates in application-level update
helpers by always setting updatedAt = Math.floor(Date.now()/1000) or new Date()
(depending on your storage format) inside the shared update function so every
update call updates the timestamp.
| sqlite.pragma('temp_store = MEMORY') // Fast temp operations | ||
| sqlite.pragma('mmap_size = 30000000000') // Memory-mapped I/O |
There was a problem hiding this comment.
✅ Addressed - Updated code to use 256MB mmap_size (embedded-friendly) and added comment about this being a 64-bit assumption. Created issue #99 to update the ADR documentation.
| CREATE TABLE `movement` ( | ||
| `id` integer PRIMARY KEY AUTOINCREMENT NOT NULL, | ||
| `side` text NOT NULL, | ||
| `timestamp` integer NOT NULL, | ||
| `total_movement` integer NOT NULL | ||
| ); |
There was a problem hiding this comment.
✅ Fixed - Converted all standalone index objects to inline index() definitions in table declarations. New migration generated with CREATE INDEX statements.
✅ Addressed CodeRabbit Review CommentsI've systematically addressed all critical and major review comments from CodeRabbit: 🔧 Critical Issues Fixed1. Database Schema Indexes (src/db/schema.ts)
2. SideSelector Component ESLint Issues (src/components/SideSelector/SideSelector.tsx)
3. Database Configuration (src/db/index.ts)
4. Migration Issues (src/db/migrate.ts)
5. Data Integrity (src/db/schema.ts)
📊 Migration GeneratedA new migration
⏭️ Next StepsThe remaining minor/nitpick suggestions from CodeRabbit (e.g., updatedAt triggers, enum CHECK constraints, ADR documentation updates) can be addressed in follow-up PRs as they are not blocking and would benefit from separate focused changes. |
📋 Follow-up Issues CreatedI've created GitHub issues for the remaining minor improvements suggested by CodeRabbit:
These improvements are valuable but non-blocking and can be addressed in focused follow-up PRs. |
🎉 All Review Comments AddressedAll CodeRabbit review comments have been systematically addressed across two commits: Commit 1: - Critical Fixes✅ Fixed 11 critical and major issues including:
Commit 2:
|
🔧 CI Configuration UpdatedRemoved all Prisma references from GitHub Actions workflows (commit Changes Made
ResultCI workflows now correctly reflect the Drizzle ORM migration. The workflow will:
This completes the full migration from Prisma to Drizzle ORM across code, configuration, and CI! 🎉 |
📝 .gitignore Updated (Commit
|
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (9)
src/components/SideSelector/SideSelector.tsx (3)
23-25: Unusedtemperature/setTemperatureandactiveTab/setActiveTabstate (duplicate)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SideSelector/SideSelector.tsx` around lines 23 - 25, The component defines unused/duplicate state variables temperature/setTemperature and activeTab/setActiveTab in SideSelector; remove these unused hooks from the SideSelector component (delete the declarations for temperature, setTemperature, activeTab, setActiveTab) or if they were intended to be used, replace the duplicate declarations with references to the single shared state used elsewhere (ensure only one source of truth for temperature and activeTab). Locate the hooks by the identifiers temperature, setTemperature, activeTab, and setActiveTab and remove the redundant useState calls and any related unused imports or props to keep the component clean.
58-58: Inconsistent stable-trend icon between left and right sides (duplicate)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SideSelector/SideSelector.tsx` at line 58, Both sides currently render the same TrendingUp icon, causing a duplicate/stable-trend visual; locate the repeated <TrendingUp ... /> usage in SideSelector.tsx and replace the right-side instance with a contrasting icon (e.g., TrendingDown or an appropriate down/decline icon from the same icon set) and update the import to include that icon instead of importing only TrendingUp. Ensure the left side keeps TrendingUp and the right side uses the new icon so the two sides are visually distinct.
10-20: Semicolon delimiters in interfaces and extra semicolon on line 27 (duplicate)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SideSelector/SideSelector.tsx` around lines 10 - 20, The interfaces SideTemp and SideSelectorProps currently have inconsistent/duplicated semicolon usage; edit the SideTemp and SideSelectorProps declarations to use a single, consistent delimiter style (either semicolons or commas) for property separators and remove the extra duplicate semicolon present after the interfaces (the stray semicolon referenced on line ~27). Ensure SideTemp (currentTemp, targetTemp) and SideSelectorProps (temperatures.leftSide, temperatures.rightSide) are cleaned up so there are no duplicate or stray semicolons.src/db/migrations/0000_plain_silverclaw.sql (1)
37-42: Missing indexes on time-series and schedule tables (duplicate)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/migrations/0000_plain_silverclaw.sql` around lines 37 - 42, The migration lacks indexes for querying time-series data: add an index on movement.timestamp (and consider one on movement.total_movement if you frequently filter/sort by it) by creating e.g. CREATE INDEX idx_movement_timestamp ON movement(timestamp); also add analogous indexes on your time-series and schedule tables (e.g. their timestamp/schedule_time columns) to speed range queries and scheduling lookups; update this SQL migration to include these CREATE INDEX statements.src/db/migrate.ts (3)
12-14: Fragile relativemigrationsFolderpath (duplicate)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/migrate.ts` around lines 12 - 14, The hard-coded relative migrationsFolder './src/db/migrations' used in the call to migrate is fragile and duplicated; change it to an absolute path built from the current module directory (e.g. use Node's path utilities with __dirname or import path and call path.resolve/path.join against __dirname and 'migrations') so migrate receives a stable, absolute migrationsFolder; update the call site that references './src/db/migrations' accordingly to remove duplication.
33-63: Seed inserts not wrapped in a transaction — partial seed possible (duplicate)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/migrate.ts` around lines 33 - 63, The seed block that checks existingSettings.length then performs multiple db.insert calls (deviceSettings, sideSettings, deviceState) must be executed inside a single transaction to avoid partial inserts/duplicates; replace the standalone awaits with a single transactional call (e.g., db.transaction or equivalent) and perform tx.insert(...) for each insert inside the transaction so all inserts commit or rollback together, keeping the existingSettings check and using the same identifiers (deviceSettings, sideSettings, deviceState, db.insert) but switched to the transaction handle.
17-17:⚠️ Potential issue | 🟠 Major
@stylistic/brace-styleESLint errors at lines 17 and 67 will fail CI.Both
} catchoccurrences (line 17 inrunMigrations, line 67 inseedDefaultData) trigger the project's brace-style rule. Thecatchblock opening must be on its own line relative to the closing}.🔧 Proposed fix (apply to both occurrences)
- } catch (error) { + } + catch (error) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/migrate.ts` at line 17, The brace-style ESLint failures are caused by the `} catch` placement; update both catch blocks so the `catch` keyword starts on its own line (i.e., move the `catch` to a new line after the closing brace) in the two functions `runMigrations` and `seedDefaultData` to satisfy the `@stylistic/brace-style` rule.src/db/schema.ts (2)
229-253: Standalone*Indexesobjects are still never applied — unresolved from previous reviewDrizzle Kit ignores these raw
sqltemplate exports; the generated migration SQL contains noCREATE INDEXstatements. The fix (inline each index into its table's third argument using theindex()builder fromdrizzle-orm/sqlite-core) was provided in detail in the prior review. This is blocking for any query that relies on those indexes for acceptable performance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/schema.ts` around lines 229 - 253, The exported standalone index objects (vitalsIndexes, movementIndexes, sleepRecordsIndexes, temperatureSchedulesIndexes, powerSchedulesIndexes, alarmSchedulesIndexes) are not picked up by Drizzle and thus not applied; update each table definition to inline these indexes using the index() builder in the table's third argument (the options object) instead of exporting raw sql templates—e.g., move the SQL from vitalsIndexes.sideTimestamp into the vitals table options via index('idx_vitals_side_timestamp', { columns: ['side','timestamp'], mode: 'desc' }) (and do the analogous change for movement, sleep_records, temperature_schedules, power_schedules, and alarm_schedules) so generated migrations include the CREATE INDEX statements.
30-32:sideSettings.namestill defaults to'Left'for both sides — unresolved from previous reviewA
'right'row inserted without an explicitnamewill store'Left'. Remove the misleading default and ensure all insert/seed paths supply the correct name.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/schema.ts` around lines 30 - 32, The sideSettings table currently sets name to default('Left') causing the 'right' row to inherit 'Left'; remove the misleading default from the name column in the sqliteTable('side_settings') definition (the sideSettings constant) and update any seed/insert code that creates rows for sideSettings to explicitly provide the correct name for both 'left' and 'right' sides so no implicit default is used.
🧹 Nitpick comments (4)
src/components/SideSelector/SideSelector.tsx (1)
45-63: IIFE pattern adds unnecessary indirection — extract a helper instead.Both buttons contain identical IIFE logic that computes
currentF,targetF,trend, andscale. A small helper component or function (e.g.,SideTempDisplay) would eliminate the duplication and the cognitive overhead of two self-invoking lambdas.♻️ Suggested refactor
+const SideTempDisplay = ({ sideData }: { sideData: SideTemp }) => { + const currentF = sideData.currentTemp + const trend = determineTrend(currentF, sideData.targetTemp) + const scale = mapToEightSleepScale(currentF) + return ( + <> + {trend === 'up' + ? <TrendingUp size={14} className="text-amber-500" /> + : trend === 'down' + ? <TrendingDown size={14} className="text-sky-500" /> + : <TrendingUp size={14} className="text-zinc-400" />} + <span> + {formatTemp(currentF, 'F')} + {' · '} + {scale} + /10 + </span> + </> + ) +} // Inside the left button: - { - ((): JSX.Element => { - // ... - })() - } + <SideTempDisplay sideData={leftSide} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SideSelector/SideSelector.tsx` around lines 45 - 63, The repeated IIFE block should be replaced by a small helper — extract a function or component (e.g., SideTempDisplay) that takes the side data (leftSide/rightSide) and returns the JSX; move the computations of currentF (ensureF), targetF (ensureF), trend (determineTrend), scale (mapToEightSleepScale) and the final formatted span (formatTemp) into that helper, then call <SideTempDisplay side={leftSide} /> and <SideTempDisplay side={rightSide} /> in place of the IIFEs to remove duplication and improve readability.src/db/migrations/0000_plain_silverclaw.sql (1)
11-12:updated_atcolumns have no auto-update trigger — values will stay stale.
DEFAULT (unixepoch())sets the timestamp at INSERT time, but SQLite has no equivalent ofON UPDATE CURRENT_TIMESTAMP. All six tables withupdated_at(alarm_schedules,device_settings,power_schedules,side_settings,tap_gestures,temperature_schedules) will retain their creation timestamp after updates unless the application layer (or a trigger) explicitly sets the column on everyUPDATE. Drizzle ORM does not do this automatically.Options:
- Add a per-table
AFTER UPDATEtrigger, e.g.:CREATE TRIGGER update_device_settings_updated_at AFTER UPDATE ON device_settings FOR EACH ROW BEGIN UPDATE device_settings SET updated_at = unixepoch() WHERE id = OLD.id; END;- Update
updatedAtexplicitly in every Drizzle mutation in the application layer (db.update(...).set({ updatedAt: sql\(unixepoch())` })`).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/migrations/0000_plain_silverclaw.sql` around lines 11 - 12, The migration defines `updated_at` with DEFAULT unixepoch() but lacks auto-update behavior, so rows in tables like alarm_schedules, device_settings, power_schedules, side_settings, tap_gestures, and temperature_schedules will keep stale timestamps; fix by adding per-table AFTER UPDATE triggers (named e.g. update_device_settings_updated_at) that set the table's updated_at = unixepoch() for the updated row, or alternatively ensure every Drizzle mutation updates updatedAt via db.update(...).set({ updatedAt: sql`(unixepoch())` }); apply the chosen approach consistently for all six tables and use the table names above to locate where to add triggers or update calls.src/db/migrate.ts (1)
8-20:better-sqlite3operations are synchronous —async/awaitis misleading.
migrate()fromdrizzle-orm/better-sqlite3/migratoris synchronous (better-sqlite3 has no async API). Wrapping it inasync+awaitmakes callers believe it is I/O-bound. The same applies todb.select()anddb.insert()inseedDefaultData. Everything resolves synchronously; the promises fulfill in the same tick. Consider removing theasyncmodifier andawaitusage, or adding a comment explaining the intentional forward-compatibility stance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/migrate.ts` around lines 8 - 20, The runMigrations function (and any seedDefaultData usage of db.select/db.insert) is marked async and awaits migrate(), but migrate from drizzle-orm/better-sqlite3 and better-sqlite3 DB ops are synchronous; remove the async modifier and any await around migrate/db.select/db.insert to avoid implying I/O, or if you intentionally want forward-compatibility, keep them but add a clear comment in runMigrations and seedDefaultData stating “migrate/db.select/db.insert are synchronous (better-sqlite3); async/await kept only for forward compatibility” and ensure you do not rely on real async behavior elsewhere.src/db/schema.ts (1)
75-85: RepeateddayOfWeekenum literal in three tables — extract a shared constantThe same 7-element tuple is copy-pasted verbatim into
temperatureSchedules,powerSchedules, andalarmSchedules. A singleconst DAY_OF_WEEKat module scope eliminates the duplication.♻️ Proposed refactor
+const DAY_OF_WEEK = [ + 'sunday', 'monday', 'tuesday', 'wednesday', 'thursday', 'friday', 'saturday', +] as const + export const temperatureSchedules = sqliteTable('temperature_schedules', { // ... dayOfWeek: text('day_of_week', { - enum: [ - 'sunday', 'monday', 'tuesday', 'wednesday', 'thursday', 'friday', 'saturday', - ], + enum: DAY_OF_WEEK, }).notNull(), // ... })Apply the same change to
powerSchedulesandalarmSchedules.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/schema.ts` around lines 75 - 85, Extract the repeated 7-item enum array used for dayOfWeek into a single module-level constant (e.g. const DAY_OF_WEEK = ['sunday','monday','tuesday','wednesday','thursday','friday','saturday']) and replace the inline enum arrays in the dayOfWeek column definitions for temperatureSchedules, powerSchedules, and alarmSchedules with a reference to DAY_OF_WEEK; ensure the new constant is exported or available in the same file and update any imports/usages if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/commands/pr-describe.md:
- Around line 25-41: The fenced code block in the PR body template
(.claude/commands/pr-describe.md) lacks a language specifier causing MD040;
update the opening triple-backtick for the template block to include a language
tag (e.g., "markdown" or "text") so the block starts with ```markdown and
suppresses the markdownlint warning while preserving the existing template
content.
In `@package.json`:
- Line 37: Update the Next.js dependency in package.json from "next": "^16.0.10"
(lockfile currently resolving 16.1.0) to "next": "^16.1.5" to pull in security
fixes (CVE-2026-23864, CVE-2025-59471, CVE-2025-59472), then run pnpm install to
regenerate the lockfile and ensure the patched 16.1.5 version is used; verify
package.json and the lockfile reflect the new version and run your test suite.
In `@src/components/SideSelector/SideSelector.tsx`:
- Around line 53-60: The JSX ternary inside SideSelector that renders
TrendingUp/TrendingDown and the span content must be reformatted to satisfy
`@stylistic/multiline-ternary` and `@stylistic/jsx-one-expression-per-line`: replace
the inline nested ternary (using TrendingUp, TrendingDown) with a multiline
conditional (each of the test, consequent, alternate on its own line) and split
the JSX inside the <span> so each expression (formatTemp(currentF, 'F'), the
middle dot, and {scale}/10) is on its own line; apply the same multiline
refactor to the right-side button IIFE (the symmetric block around lines 87–94),
and add a final newline at EOF to satisfy `@stylistic/eol-last`.
- Around line 47-60: SideSelector currently forces Fahrenheit by calling
ensureF(...) and formatTemp(..., 'F'); add a unit prop to SideSelector and use
it to display and compare temperatures: keep ensureF(leftSide.currentTemp) and
ensureF(leftSide.targetTemp) to get currentF/targetF (these are needed for
mapToEightSleepScale and any DB-consistent logic), then if unit === 'C' convert
those F values to Celsius for display and trend comparison (but continue to pass
the F value into mapToEightSleepScale), call determineTrend on the values in the
same display unit, and call formatTemp(displayValue, unit) instead of hardcoding
'F'; apply the same changes for the right-side block (references: ensureF,
determineTrend, mapToEightSleepScale, formatTemp, SideSelector,
leftSide/rightSide).
- Line 46: The IIFE uses a global JSX type that is unavailable under React 19/TS
5.9.3; fix by either removing the explicit return-type annotation on the
anonymous IIFE (let TypeScript infer the type) or import React and change the
annotation to use React.JSX.Element (e.g., replace "(): JSX.Element =>" with
"(): React.JSX.Element =>" after importing React). Update the occurrence(s) in
the SideSelector component where the IIFE/arrow function has the JSX.Element
return-type (the anonymous IIFE starting with "(): JSX.Element =>") so the code
compiles under scoped-jsx.
In `@src/db/migrate.ts`:
- Line 74: The current direct comparison if (import.meta.url ===
`file://${process.argv[1]}`) is fragile on Windows; change it to compare
normalized filesystem paths by converting the file URL to a path using
fileURLToPath(import.meta.url) and resolving process.argv[1] via
path.resolve(process.argv[1]). Import fileURLToPath from 'url' and path from
'path', then replace the condition with fileURLToPath(import.meta.url) ===
path.resolve(process.argv[1]) so the check works consistently across platforms.
In `@src/db/migrations/0000_plain_silverclaw.sql`:
- Around line 56-62: The side_settings table defines name with a hardcoded
DEFAULT 'Left', which causes right-side rows to get the wrong name; update the
schema so name is not defaulted to 'Left'—either remove the DEFAULT from column
side_settings.name and enforce presence/validation in the application, or
replace the default with a DB-side rule (a CHECK constraint or a trigger on
INSERT that sets name = CASE WHEN side='left' THEN 'Left' WHEN side='right' THEN
'Right' END) so the name is derived from side; adjust any seed/migration logic
that relies on the previous default accordingly.
In `@src/db/schema.ts`:
- Around line 177-189: Change the sleep_records schema so in-progress sessions
can be created and duration isn’t stored inconsistently: make leftBedAt (column
left_bed_at / symbol leftBedAt) nullable and make sleepDurationSeconds
(sleep_duration_seconds / symbol sleepDurationSeconds) nullable as well; then
either remove the sleepDurationSeconds column and compute duration as leftBedAt
- enteredBedAt on reads, or keep it but add an explicit consistency enforcement
(a DB CHECK constraint or application-level assertion in your insert/update code
for sleepRecords that ensures sleepDurationSeconds == leftBedAt - enteredBedAt
whenever leftBedAt is set).
- Around line 42-66: The tapGestures table allows mismatched action-specific
fields because actionType isn't enforced; add a DB-level CHECK constraint in the
tapGestures table definition (using Drizzle's sql helper) that validates when
actionType = 'temperature' then temperatureChange and temperatureAmount are
non-NULL and alarmBehavior, alarmSnoozeDuration, alarmInactiveBehavior are NULL,
and when actionType = 'alarm' then alarmBehavior is non-NULL (and optionally
alarmSnoozeDuration/alarmInactiveBehavior as appropriate) and
temperatureChange/temperatureAmount are NULL; update the tapGestures table
declaration to include this sql`CHECK(...)` expression so the database enforces
the contract for actionType/field combinations.
- Around line 211-223: The systemHealth table as defined
(sqliteTable('system_health') with columns id, component, status, message,
lastChecked) lacks a uniqueness constraint on component so repeated health
checks will append rows; change the schema to enforce one-row-per-component by
making component the primary key (remove/stop using autoIncrement id) or add a
UNIQUE constraint on component and switch write code to use upsert/ON CONFLICT
to update lastChecked/status in the systemHealth table (refer to systemHealth,
component, id, lastChecked); alternatively, if you want audit behavior keep id
autoIncrement but add a retention policy/job to prune old rows and document the
intent.
- Around line 25-27: Each table's updatedAt column currently uses
.default(sql`(unixepoch())`) which only sets the value on INSERT; for
deviceSettings.updatedAt, sideSettings.updatedAt, tapGestures.updatedAt,
temperatureSchedules.updatedAt, powerSchedules.updatedAt, and
alarmSchedules.updatedAt add a .$onUpdate(() => new Date()) callback so the
column is updated on UPDATE operations; do not use a raw sql`` expression in
$onUpdate (use new Date()) to avoid the toISOString TypeError.
---
Duplicate comments:
In `@src/components/SideSelector/SideSelector.tsx`:
- Around line 23-25: The component defines unused/duplicate state variables
temperature/setTemperature and activeTab/setActiveTab in SideSelector; remove
these unused hooks from the SideSelector component (delete the declarations for
temperature, setTemperature, activeTab, setActiveTab) or if they were intended
to be used, replace the duplicate declarations with references to the single
shared state used elsewhere (ensure only one source of truth for temperature and
activeTab). Locate the hooks by the identifiers temperature, setTemperature,
activeTab, and setActiveTab and remove the redundant useState calls and any
related unused imports or props to keep the component clean.
- Line 58: Both sides currently render the same TrendingUp icon, causing a
duplicate/stable-trend visual; locate the repeated <TrendingUp ... /> usage in
SideSelector.tsx and replace the right-side instance with a contrasting icon
(e.g., TrendingDown or an appropriate down/decline icon from the same icon set)
and update the import to include that icon instead of importing only TrendingUp.
Ensure the left side keeps TrendingUp and the right side uses the new icon so
the two sides are visually distinct.
- Around line 10-20: The interfaces SideTemp and SideSelectorProps currently
have inconsistent/duplicated semicolon usage; edit the SideTemp and
SideSelectorProps declarations to use a single, consistent delimiter style
(either semicolons or commas) for property separators and remove the extra
duplicate semicolon present after the interfaces (the stray semicolon referenced
on line ~27). Ensure SideTemp (currentTemp, targetTemp) and SideSelectorProps
(temperatures.leftSide, temperatures.rightSide) are cleaned up so there are no
duplicate or stray semicolons.
In `@src/db/migrate.ts`:
- Around line 12-14: The hard-coded relative migrationsFolder
'./src/db/migrations' used in the call to migrate is fragile and duplicated;
change it to an absolute path built from the current module directory (e.g. use
Node's path utilities with __dirname or import path and call
path.resolve/path.join against __dirname and 'migrations') so migrate receives a
stable, absolute migrationsFolder; update the call site that references
'./src/db/migrations' accordingly to remove duplication.
- Around line 33-63: The seed block that checks existingSettings.length then
performs multiple db.insert calls (deviceSettings, sideSettings, deviceState)
must be executed inside a single transaction to avoid partial
inserts/duplicates; replace the standalone awaits with a single transactional
call (e.g., db.transaction or equivalent) and perform tx.insert(...) for each
insert inside the transaction so all inserts commit or rollback together,
keeping the existingSettings check and using the same identifiers
(deviceSettings, sideSettings, deviceState, db.insert) but switched to the
transaction handle.
- Line 17: The brace-style ESLint failures are caused by the `} catch`
placement; update both catch blocks so the `catch` keyword starts on its own
line (i.e., move the `catch` to a new line after the closing brace) in the two
functions `runMigrations` and `seedDefaultData` to satisfy the
`@stylistic/brace-style` rule.
In `@src/db/migrations/0000_plain_silverclaw.sql`:
- Around line 37-42: The migration lacks indexes for querying time-series data:
add an index on movement.timestamp (and consider one on movement.total_movement
if you frequently filter/sort by it) by creating e.g. CREATE INDEX
idx_movement_timestamp ON movement(timestamp); also add analogous indexes on
your time-series and schedule tables (e.g. their timestamp/schedule_time
columns) to speed range queries and scheduling lookups; update this SQL
migration to include these CREATE INDEX statements.
In `@src/db/schema.ts`:
- Around line 229-253: The exported standalone index objects (vitalsIndexes,
movementIndexes, sleepRecordsIndexes, temperatureSchedulesIndexes,
powerSchedulesIndexes, alarmSchedulesIndexes) are not picked up by Drizzle and
thus not applied; update each table definition to inline these indexes using the
index() builder in the table's third argument (the options object) instead of
exporting raw sql templates—e.g., move the SQL from vitalsIndexes.sideTimestamp
into the vitals table options via index('idx_vitals_side_timestamp', { columns:
['side','timestamp'], mode: 'desc' }) (and do the analogous change for movement,
sleep_records, temperature_schedules, power_schedules, and alarm_schedules) so
generated migrations include the CREATE INDEX statements.
- Around line 30-32: The sideSettings table currently sets name to
default('Left') causing the 'right' row to inherit 'Left'; remove the misleading
default from the name column in the sqliteTable('side_settings') definition (the
sideSettings constant) and update any seed/insert code that creates rows for
sideSettings to explicitly provide the correct name for both 'left' and 'right'
sides so no implicit default is used.
---
Nitpick comments:
In `@src/components/SideSelector/SideSelector.tsx`:
- Around line 45-63: The repeated IIFE block should be replaced by a small
helper — extract a function or component (e.g., SideTempDisplay) that takes the
side data (leftSide/rightSide) and returns the JSX; move the computations of
currentF (ensureF), targetF (ensureF), trend (determineTrend), scale
(mapToEightSleepScale) and the final formatted span (formatTemp) into that
helper, then call <SideTempDisplay side={leftSide} /> and <SideTempDisplay
side={rightSide} /> in place of the IIFEs to remove duplication and improve
readability.
In `@src/db/migrate.ts`:
- Around line 8-20: The runMigrations function (and any seedDefaultData usage of
db.select/db.insert) is marked async and awaits migrate(), but migrate from
drizzle-orm/better-sqlite3 and better-sqlite3 DB ops are synchronous; remove the
async modifier and any await around migrate/db.select/db.insert to avoid
implying I/O, or if you intentionally want forward-compatibility, keep them but
add a clear comment in runMigrations and seedDefaultData stating
“migrate/db.select/db.insert are synchronous (better-sqlite3); async/await kept
only for forward compatibility” and ensure you do not rely on real async
behavior elsewhere.
In `@src/db/migrations/0000_plain_silverclaw.sql`:
- Around line 11-12: The migration defines `updated_at` with DEFAULT unixepoch()
but lacks auto-update behavior, so rows in tables like alarm_schedules,
device_settings, power_schedules, side_settings, tap_gestures, and
temperature_schedules will keep stale timestamps; fix by adding per-table AFTER
UPDATE triggers (named e.g. update_device_settings_updated_at) that set the
table's updated_at = unixepoch() for the updated row, or alternatively ensure
every Drizzle mutation updates updatedAt via db.update(...).set({ updatedAt:
sql`(unixepoch())` }); apply the chosen approach consistently for all six tables
and use the table names above to locate where to add triggers or update calls.
In `@src/db/schema.ts`:
- Around line 75-85: Extract the repeated 7-item enum array used for dayOfWeek
into a single module-level constant (e.g. const DAY_OF_WEEK =
['sunday','monday','tuesday','wednesday','thursday','friday','saturday']) and
replace the inline enum arrays in the dayOfWeek column definitions for
temperatureSchedules, powerSchedules, and alarmSchedules with a reference to
DAY_OF_WEEK; ensure the new constant is exported or available in the same file
and update any imports/usages if needed.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
generated/prisma/browser.tsis excluded by!**/generated/**generated/prisma/client.tsis excluded by!**/generated/**generated/prisma/commonInputTypes.tsis excluded by!**/generated/**generated/prisma/enums.tsis excluded by!**/generated/**generated/prisma/internal/class.tsis excluded by!**/generated/**generated/prisma/internal/prismaNamespace.tsis excluded by!**/generated/**generated/prisma/internal/prismaNamespaceBrowser.tsis excluded by!**/generated/**generated/prisma/models.tsis excluded by!**/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
.claude/commands/pr-describe.md.claude/docs/PROJECT_INFO.mddocs/adr/0010-drizzle-orm-sqlite.mddocs/adr/0010-prisma-sqlite.mddrizzle.config.tspackage.jsonprisma.config.tsprisma/schema.prismasrc/components/SideSelector/SideSelector.tsxsrc/db/index.tssrc/db/migrate.tssrc/db/migrations/0000_plain_silverclaw.sqlsrc/db/migrations/meta/0000_snapshot.jsonsrc/db/migrations/meta/_journal.jsonsrc/db/schema.tssrc/lib/prisma.tssrc/lib/tempUtils.ts
💤 Files with no reviewable changes (4)
- prisma.config.ts
- prisma/schema.prisma
- docs/adr/0010-prisma-sqlite.md
- src/lib/prisma.ts
✅ Files skipped from review due to trivial changes (1)
- .claude/docs/PROJECT_INFO.md
🚧 Files skipped from review as they are similar to previous changes (3)
- src/db/index.ts
- drizzle.config.ts
- docs/adr/0010-drizzle-orm-sqlite.md
| ``` | ||
| ## Summary | ||
|
|
||
| - Bullet point summary of major changes | ||
| - Focus on *what* changed and *why* | ||
| - Use emojis sparingly to improve scannability | ||
|
|
||
| ## Scope Breakdown | ||
|
|
||
| | Area | Summary | | ||
| |------|---------| | ||
| | relevant/path/ | Brief description of changes in that area | | ||
|
|
||
| ## Test plan | ||
|
|
||
| - [ ] Checklist of what should be tested | ||
| ``` |
There was a problem hiding this comment.
Add a language specifier to the fenced code block.
The PR body template block has no language tag, causing the MD040 markdownlint warning. Use markdown or text to suppress it:
📝 Proposed fix
-```
+```markdown
## Summary🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 25-25: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/commands/pr-describe.md around lines 25 - 41, The fenced code block
in the PR body template (.claude/commands/pr-describe.md) lacks a language
specifier causing MD040; update the opening triple-backtick for the template
block to include a language tag (e.g., "markdown" or "text") so the block starts
with ```markdown and suppresses the markdownlint warning while preserving the
existing template content.
| "drizzle-orm": "^0.45.1", | ||
| "lucide-react": "^0.562.0", | ||
| "negotiator": "^1.0.0", | ||
| "next": "^16.0.10", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
grep -A 5 '"next"' pnpm-lock.yaml | head -20Repository: sleepypod/core
Length of output: 40
🌐 Web query:
CVE-2025-55182 CVE-2025-66478 Next.js RCE deserialization Flight protocol
💡 Result:
- CVE-2025-55182 (“React2Shell”): A critical (CVSS 10.0) pre-auth RCE caused by unsafe deserialization in React Server Components (RSC) “Flight” protocol handling. A crafted HTTP request can reach React/Next.js server-side RSC deserialization and lead to arbitrary code execution. Affected React RSC packages/versions:
react-server-dom-webpack,react-server-dom-parcel,react-server-dom-turbopackin 19.0.0, 19.1.0, 19.1.1, 19.2.0. Patched React versions: 19.0.1, 19.1.2, 19.2.1. [1][2][3] - Next.js impact: Next.js App Router deployments that use RSC inherit the issue (reachable in typical configurations). Affected Next.js ranges include 14.3.0-canary.77+ canaries, and 15.x / 16.x up to specific patch releases; fixed in 15.0.5, 15.1.9, 15.2.6, 15.3.6, 15.4.8, 15.5.7, 16.0.7 (or later). [3][4]
- CVE-2025-66478: Originally used to track the Next.js downstream exposure, but it has been REJECTED as a duplicate of CVE-2025-55182 (so treat 55182 as the canonical CVE). [3][5]
- How exploitation is commonly described: Attackers send specially crafted requests that look like/target RSC/Flight (“server function” / Server Actions) endpoints, triggering the vulnerable deserialization before app-level auth/logic. [1][4]
What to do
- Upgrade React and Next.js to the patched versions above (both matter). [3][4]
- If you can’t patch immediately, use WAF/edge rules as a temporary control and monitor for exploit attempts. [4]
Sources: Wiz technical write-up [1], Microsoft defender guidance (affected/patched versions) [4], AWS security bulletin (duplicate CVE note + affected stacks) [3], Snyk Next.js advisory (fixed versions) [2], NVD rejection notice for CVE-2025-66478 [5].
[1] https://www.wiz.io/blog/critical-vulnerability-in-react-cve-2025-55182
[2] https://security.snyk.io/vuln/SNYK-JS-NEXT-14173355
[3] https://aws.amazon.com/security/security-bulletins/rss/aws-2025-030/
[4] https://www.microsoft.com/en-us/security/blog/2025/12/15/defending-against-the-cve-2025-55182-react2shell-vulnerability-in-react-server-components/
[5] https://nvd.nist.gov/vuln/detail/CVE-2025-66478
🌐 Web query:
Next.js 16.1 security patches release notes
💡 Result:
Next.js 16.1 security patch release notes (stable)
Next.js v16.1.5 (released Jan 26, 2026) — security release
GitHub release notes for v16.1.5 point to the official Vercel security summaries for what changed/fixed. [1]
Fixes included:
- CVE-2026-23864 (High, CVSS 7.5) — DoS in React Server Components (RSC) / App Router Server Function endpoints; patched in Next.js 16.1.5 (and other listed versions). [2][4]
- CVE-2025-59471 (CVSS 5.9) — DoS via memory exhaustion in Image Optimizer when external images are enabled via
remotePatterns; Next.js >=16 and <16.1.5 affected. [3] - CVE-2025-59472 (CVSS 5.9) — DoS via memory exhaustion in PPR resume endpoint under specific configs (PPR/minimal mode); Next.js >=16 and <16.1.5 affected. [3]
Affected → patched (16.1 line):
- Affected:
>=16.0.0 <16.1.5→ Patched:16.1.5. [4]
Note on v16.1.4
v16.1.4 (released Jan 19, 2026) is described as a bug-fix backport release (not the security patch release for the CVEs above). [5]
Sources:
[1] (github.com)
[2] (vercel.com)
[3] (vercel.com)
[4] (github.com)
[5] (github.com)
Citations:
- 1: https://github.com/vercel/next.js/releases/tag/v16.1.5
- 2: https://vercel.com/changelog/summary-of-cve-2026-23864
- 3: https://vercel.com/changelog/summaries-of-cve-2025-59471-and-cve-2025-59472
- 4: GHSA-h25m-26qc-wcjf
- 5: https://github.com/vercel/next.js/releases/tag/v16.1.4
🏁 Script executed:
test -f pnpm-lock.yaml && echo "File exists" || echo "File not found"Repository: sleepypod/core
Length of output: 69
🏁 Script executed:
find . -name "pnpm-lock.yaml" -o -name "package-lock.json" -o -name "yarn.lock" 2>/dev/null | head -5Repository: sleepypod/core
Length of output: 138
🏁 Script executed:
if [ -f pnpm-lock.yaml ]; then
cat pnpm-lock.yaml | grep -A 20 "next@" | head -30
fiRepository: sleepypod/core
Length of output: 1899
Upgrade Next.js to 16.1.5 or later — multiple unpatched security issues in current 16.1.0 lockfile version.
The lockfile pins next@16.1.0, which is missing patches for:
- CVE-2026-23864 (High): DoS in React Server Components/Server Function endpoints
- CVE-2025-59471 (Medium): DoS via Image Optimizer memory exhaustion
- CVE-2025-59472 (Medium): DoS via PPR resume endpoint memory exhaustion
All three are fixed in Next.js 16.1.5 (released Jan 26, 2026). Update package.json to ^16.1.5 and run pnpm install to pick up the patch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 37, Update the Next.js dependency in package.json from
"next": "^16.0.10" (lockfile currently resolving 16.1.0) to "next": "^16.1.5" to
pull in security fixes (CVE-2026-23864, CVE-2025-59471, CVE-2025-59472), then
run pnpm install to regenerate the lockfile and ensure the patched 16.1.5
version is used; verify package.json and the lockfile reflect the new version
and run your test suite.
| const currentF = ensureF(leftSide.currentTemp, 'F') | ||
| const targetF = ensureF(leftSide.targetTemp, 'F') | ||
| const trend = determineTrend(currentF, targetF) | ||
| const scale = mapToEightSleepScale(currentF) | ||
| return ( | ||
| <> | ||
| {trend === 'up' ? ( | ||
| <TrendingUp size={14} className="text-amber-500" /> | ||
| ) : trend === 'down' ? ( | ||
| <TrendingDown size={14} className="text-sky-500" /> | ||
| ) : ( | ||
| <TrendingUp size={14} className="text-zinc-400" /> | ||
| )} | ||
| <span>{formatTemp(currentF, 'F')} · {scale}/10</span> |
There was a problem hiding this comment.
Temperatures always displayed in °F — ignores user's temperatureUnit preference.
ensureF(leftSide.currentTemp, 'F') is a no-op (the DB already stores in °F per schema.ts comments), and formatTemp(currentF, 'F') hardcodes the °F suffix and scale. If a user has configured temperatureUnit: 'C' in device_settings, they will see Fahrenheit values with a °F label. Same bug at lines 81–82/94 for the right side.
The component should accept a unit prop (from device settings) and apply the appropriate conversion and label:
🔧 Proposed fix — add unit prop and convert for display
+import { TempUnit, toC, formatTemp, determineTrend, mapToEightSleepScale } from 'src/lib/tempUtils'
interface SideSelectorProps {
temperatures: {
leftSide: SideTemp
rightSide: SideTemp
- }
+ }
+ unit?: TempUnit
}
-export const SideSelector = ({ temperatures }: SideSelectorProps) => {
+export const SideSelector = ({ temperatures, unit = 'F' }: SideSelectorProps) => {
// ...
// Inside the IIFE (or helper function):
- const currentF = ensureF(leftSide.currentTemp, 'F')
+ const currentF = leftSide.currentTemp // already stored in °F
+ const displayTemp = unit === 'C' ? toC(currentF) : currentF
// ...
- <span>{formatTemp(currentF, 'F')} · {scale}/10</span>
+ <span>{formatTemp(displayTemp, unit)} · {scale}/10</span>🧰 Tools
🪛 ESLint
[error] 53-53: Expected newline between test and consequent of ternary expression.
(@stylistic/multiline-ternary)
[error] 53-55: Expected newline between consequent and alternate of ternary expression.
(@stylistic/multiline-ternary)
[error] 55-55: Expected newline between test and consequent of ternary expression.
(@stylistic/multiline-ternary)
[error] 55-57: Expected newline between consequent and alternate of ternary expression.
(@stylistic/multiline-ternary)
[error] 60-60: {formatTemp(currentF, 'F')} must be placed on a new line
(@stylistic/jsx-one-expression-per-line)
[error] 60-60: · must be placed on a new line
(@stylistic/jsx-one-expression-per-line)
[error] 60-60: {scale} must be placed on a new line
(@stylistic/jsx-one-expression-per-line)
[error] 60-60: /10 must be placed on a new line
(@stylistic/jsx-one-expression-per-line)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/SideSelector/SideSelector.tsx` around lines 47 - 60,
SideSelector currently forces Fahrenheit by calling ensureF(...) and
formatTemp(..., 'F'); add a unit prop to SideSelector and use it to display and
compare temperatures: keep ensureF(leftSide.currentTemp) and
ensureF(leftSide.targetTemp) to get currentF/targetF (these are needed for
mapToEightSleepScale and any DB-consistent logic), then if unit === 'C' convert
those F values to Celsius for display and trend comparison (but continue to pass
the F value into mapToEightSleepScale), call determineTrend on the values in the
same display unit, and call formatTemp(displayValue, unit) instead of hardcoding
'F'; apply the same changes for the right-side block (references: ensureF,
determineTrend, mapToEightSleepScale, formatTemp, SideSelector,
leftSide/rightSide).
| CREATE TABLE `side_settings` ( | ||
| `side` text PRIMARY KEY NOT NULL, | ||
| `name` text DEFAULT 'Left' NOT NULL, | ||
| `away_mode` integer DEFAULT false NOT NULL, | ||
| `created_at` integer DEFAULT (unixepoch()) NOT NULL, | ||
| `updated_at` integer DEFAULT (unixepoch()) NOT NULL | ||
| ); |
There was a problem hiding this comment.
side_settings.name default 'Left' is wrong for right-side records.
Any insert into side_settings that omits the name column will inherit 'Left' regardless of the side value. The seed in migrate.ts explicitly sets both names correctly, but ad-hoc inserts or future code paths could silently produce a right-side record named 'Left'. Consider removing the column default and making it required in the application layer, or using a CHECK constraint / trigger to derive the default from side.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/db/migrations/0000_plain_silverclaw.sql` around lines 56 - 62, The
side_settings table defines name with a hardcoded DEFAULT 'Left', which causes
right-side rows to get the wrong name; update the schema so name is not
defaulted to 'Left'—either remove the DEFAULT from column side_settings.name and
enforce presence/validation in the application, or replace the default with a
DB-side rule (a CHECK constraint or a trigger on INSERT that sets name = CASE
WHEN side='left' THEN 'Left' WHEN side='right' THEN 'Right' END) so the name is
derived from side; adjust any seed/migration logic that relies on the previous
default accordingly.
| updatedAt: integer('updated_at', { mode: 'timestamp' }) | ||
| .notNull() | ||
| .default(sql`(unixepoch())`), |
There was a problem hiding this comment.
updatedAt only captures insert time — add .$onUpdate() to all six tables
.default(sql(unixepoch())) fires exclusively on INSERT. Any subsequent UPDATE via Drizzle leaves the column frozen at the original insert timestamp. Drizzle provides $onUpdate() / $onUpdateFn() which adds a dynamic update value: the function is called when a row is updated and the returned value is used as the column value.
This affects every updatedAt column in: deviceSettings, sideSettings, tapGestures, temperatureSchedules, powerSchedules, and alarmSchedules.
🛠️ Proposed fix (shown for `deviceSettings`, apply the same pattern to all six tables)
updatedAt: integer('updated_at', { mode: 'timestamp' })
.notNull()
- .default(sql`(unixepoch())`)
+ .default(sql`(unixepoch())`)
+ .$onUpdate(() => new Date())Use
new Date()(not asql`` template) in the$onUpdatecallback forinteger({ mode: 'timestamp' })columns — passing a rawsql`` expression causes aTypeError: value.toISOString is not a functionat runtime.
📝 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.
| updatedAt: integer('updated_at', { mode: 'timestamp' }) | |
| .notNull() | |
| .default(sql`(unixepoch())`), | |
| updatedAt: integer('updated_at', { mode: 'timestamp' }) | |
| .notNull() | |
| .default(sql`(unixepoch())`) | |
| .$onUpdate(() => new Date()), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/db/schema.ts` around lines 25 - 27, Each table's updatedAt column
currently uses .default(sql`(unixepoch())`) which only sets the value on INSERT;
for deviceSettings.updatedAt, sideSettings.updatedAt, tapGestures.updatedAt,
temperatureSchedules.updatedAt, powerSchedules.updatedAt, and
alarmSchedules.updatedAt add a .$onUpdate(() => new Date()) callback so the
column is updated on UPDATE operations; do not use a raw sql`` expression in
$onUpdate (use new Date()) to avoid the toISOString TypeError.
| export const tapGestures = sqliteTable('tap_gestures', { | ||
| id: integer('id').primaryKey({ autoIncrement: true }), | ||
| side: text('side', { enum: ['left', 'right'] }).notNull(), | ||
| tapType: text('tap_type', { | ||
| enum: ['doubleTap', 'tripleTap', 'quadTap'], | ||
| }).notNull(), | ||
| actionType: text('action_type', { enum: ['temperature', 'alarm'] }).notNull(), | ||
| // For temperature actions | ||
| temperatureChange: text('temperature_change', { | ||
| enum: ['increment', 'decrement'], | ||
| }), | ||
| temperatureAmount: integer('temperature_amount'), // 0-10 | ||
| // For alarm actions | ||
| alarmBehavior: text('alarm_behavior', { enum: ['snooze', 'dismiss'] }), | ||
| alarmSnoozeDuration: integer('alarm_snooze_duration'), // 60-600 seconds | ||
| alarmInactiveBehavior: text('alarm_inactive_behavior', { | ||
| enum: ['power', 'none'], | ||
| }), | ||
| createdAt: integer('created_at', { mode: 'timestamp' }) | ||
| .notNull() | ||
| .default(sql`(unixepoch())`), | ||
| updatedAt: integer('updated_at', { mode: 'timestamp' }) | ||
| .notNull() | ||
| .default(sql`(unixepoch())`), | ||
| }) |
There was a problem hiding this comment.
No DB-level guard ensuring action-specific fields match actionType
temperatureChange / temperatureAmount and alarmBehavior / alarmSnoozeDuration / alarmInactiveBehavior are all nullable, so a row with actionType = 'temperature' can legally store alarm fields (and vice versa) with no error. SQLite supports CHECK constraints — consider adding one to enforce the contract at the schema level, or document that validation is exclusively the application's responsibility.
💡 Example CHECK constraint via Drizzle's `sql` helper
-export const tapGestures = sqliteTable('tap_gestures', {
+export const tapGestures = sqliteTable('tap_gestures', {
// ...existing columns...
+}, (t) => [
+ sql`CHECK (
+ (action_type = 'temperature' AND temperature_change IS NOT NULL AND temperature_amount IS NOT NULL)
+ OR
+ (action_type = 'alarm' AND alarm_behavior IS NOT NULL)
+ )`,
+])📝 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.
| export const tapGestures = sqliteTable('tap_gestures', { | |
| id: integer('id').primaryKey({ autoIncrement: true }), | |
| side: text('side', { enum: ['left', 'right'] }).notNull(), | |
| tapType: text('tap_type', { | |
| enum: ['doubleTap', 'tripleTap', 'quadTap'], | |
| }).notNull(), | |
| actionType: text('action_type', { enum: ['temperature', 'alarm'] }).notNull(), | |
| // For temperature actions | |
| temperatureChange: text('temperature_change', { | |
| enum: ['increment', 'decrement'], | |
| }), | |
| temperatureAmount: integer('temperature_amount'), // 0-10 | |
| // For alarm actions | |
| alarmBehavior: text('alarm_behavior', { enum: ['snooze', 'dismiss'] }), | |
| alarmSnoozeDuration: integer('alarm_snooze_duration'), // 60-600 seconds | |
| alarmInactiveBehavior: text('alarm_inactive_behavior', { | |
| enum: ['power', 'none'], | |
| }), | |
| createdAt: integer('created_at', { mode: 'timestamp' }) | |
| .notNull() | |
| .default(sql`(unixepoch())`), | |
| updatedAt: integer('updated_at', { mode: 'timestamp' }) | |
| .notNull() | |
| .default(sql`(unixepoch())`), | |
| }) | |
| export const tapGestures = sqliteTable('tap_gestures', { | |
| id: integer('id').primaryKey({ autoIncrement: true }), | |
| side: text('side', { enum: ['left', 'right'] }).notNull(), | |
| tapType: text('tap_type', { | |
| enum: ['doubleTap', 'tripleTap', 'quadTap'], | |
| }).notNull(), | |
| actionType: text('action_type', { enum: ['temperature', 'alarm'] }).notNull(), | |
| // For temperature actions | |
| temperatureChange: text('temperature_change', { | |
| enum: ['increment', 'decrement'], | |
| }), | |
| temperatureAmount: integer('temperature_amount'), // 0-10 | |
| // For alarm actions | |
| alarmBehavior: text('alarm_behavior', { enum: ['snooze', 'dismiss'] }), | |
| alarmSnoozeDuration: integer('alarm_snooze_duration'), // 60-600 seconds | |
| alarmInactiveBehavior: text('alarm_inactive_behavior', { | |
| enum: ['power', 'none'], | |
| }), | |
| createdAt: integer('created_at', { mode: 'timestamp' }) | |
| .notNull() | |
| .default(sql`(unixepoch())`), | |
| updatedAt: integer('updated_at', { mode: 'timestamp' }) | |
| .notNull() | |
| .default(sql`(unixepoch())`), | |
| }, (t) => [ | |
| sql`CHECK ( | |
| (action_type = 'temperature' AND temperature_change IS NOT NULL AND temperature_amount IS NOT NULL) | |
| OR | |
| (action_type = 'alarm' AND alarm_behavior IS NOT NULL) | |
| )`, | |
| ]) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/db/schema.ts` around lines 42 - 66, The tapGestures table allows
mismatched action-specific fields because actionType isn't enforced; add a
DB-level CHECK constraint in the tapGestures table definition (using Drizzle's
sql helper) that validates when actionType = 'temperature' then
temperatureChange and temperatureAmount are non-NULL and alarmBehavior,
alarmSnoozeDuration, alarmInactiveBehavior are NULL, and when actionType =
'alarm' then alarmBehavior is non-NULL (and optionally
alarmSnoozeDuration/alarmInactiveBehavior as appropriate) and
temperatureChange/temperatureAmount are NULL; update the tapGestures table
declaration to include this sql`CHECK(...)` expression so the database enforces
the contract for actionType/field combinations.
| export const sleepRecords = sqliteTable('sleep_records', { | ||
| id: integer('id').primaryKey({ autoIncrement: true }), | ||
| side: text('side', { enum: ['left', 'right'] }).notNull(), | ||
| enteredBedAt: integer('entered_bed_at', { mode: 'timestamp' }).notNull(), | ||
| leftBedAt: integer('left_bed_at', { mode: 'timestamp' }).notNull(), | ||
| sleepDurationSeconds: integer('sleep_duration_seconds').notNull(), | ||
| timesExitedBed: integer('times_exited_bed').notNull().default(0), | ||
| presentIntervals: text('present_intervals', { mode: 'json' }), // Array of [start, end] timestamps | ||
| notPresentIntervals: text('not_present_intervals', { mode: 'json' }), // Array of [start, end] timestamps | ||
| createdAt: integer('created_at', { mode: 'timestamp' }) | ||
| .notNull() | ||
| .default(sql`(unixepoch())`), | ||
| }) |
There was a problem hiding this comment.
leftBedAt NOT NULL prevents in-progress sessions; sleepDurationSeconds is a denormalized derived value
Two related design concerns:
-
leftBedAt: ...notNull()(line 181) andsleepDurationSeconds: ...notNull()(line 182) mean a session can only be inserted once it is fully complete. If the device needs to open a session record when someone enters bed and close it on exit, both fields need to be nullable. -
sleepDurationSecondsis fully computable asleftBedAt - enteredBedAt. Storing it separately creates a potential consistency hazard — if either timestamp is later corrected, the duration silently disagrees. Consider removing it and computing on read, or adding an application-level assertion that enforcessleepDurationSeconds == leftBedAt - enteredBedAton every write.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/db/schema.ts` around lines 177 - 189, Change the sleep_records schema so
in-progress sessions can be created and duration isn’t stored inconsistently:
make leftBedAt (column left_bed_at / symbol leftBedAt) nullable and make
sleepDurationSeconds (sleep_duration_seconds / symbol sleepDurationSeconds)
nullable as well; then either remove the sleepDurationSeconds column and compute
duration as leftBedAt - enteredBedAt on reads, or keep it but add an explicit
consistency enforcement (a DB CHECK constraint or application-level assertion in
your insert/update code for sleepRecords that ensures sleepDurationSeconds ==
leftBedAt - enteredBedAt whenever leftBedAt is set).
| export const systemHealth = sqliteTable('system_health', { | ||
| id: integer('id').primaryKey({ autoIncrement: true }), | ||
| component: text('component').notNull(), // e.g., 'express', 'database', 'franken' | ||
| status: text('status', { | ||
| enum: ['healthy', 'degraded', 'down', 'unknown'], | ||
| }) | ||
| .notNull() | ||
| .default('unknown'), | ||
| message: text('message'), | ||
| lastChecked: integer('last_checked', { mode: 'timestamp' }) | ||
| .notNull() | ||
| .default(sql`(unixepoch())`), | ||
| }) |
There was a problem hiding this comment.
systemHealth has no UNIQUE constraint on component — unbounded row growth on embedded storage
Every health check produces a new row. The field name lastChecked signals this is meant as a "current status" table, not an audit log — but without a unique constraint there's no way to enforce one-row-per-component, and no upsert semantics are possible.
On an embedded device with constrained flash/SSD this is a critical storage concern: a health check that runs every few seconds against 3–4 components will accumulate millions of rows quickly.
Two possible fixes:
- Status store (recommended): Make
componentthe primary key and always upsert.
🛠️ Option 1 — component as primary key
export const systemHealth = sqliteTable('system_health', {
- id: integer('id').primaryKey({ autoIncrement: true }),
- component: text('component').notNull(),
+ component: text('component').primaryKey(),
status: text('status', {
enum: ['healthy', 'degraded', 'down', 'unknown'],
})
.notNull()
.default('unknown'),
message: text('message'),
lastChecked: integer('last_checked', { mode: 'timestamp' })
.notNull()
.default(sql`(unixepoch())`),
})- Audit log: Keep the auto-increment PK but add a retention mechanism (e.g. a scheduled DELETE of rows older than N hours) and document the intent.
📝 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.
| export const systemHealth = sqliteTable('system_health', { | |
| id: integer('id').primaryKey({ autoIncrement: true }), | |
| component: text('component').notNull(), // e.g., 'express', 'database', 'franken' | |
| status: text('status', { | |
| enum: ['healthy', 'degraded', 'down', 'unknown'], | |
| }) | |
| .notNull() | |
| .default('unknown'), | |
| message: text('message'), | |
| lastChecked: integer('last_checked', { mode: 'timestamp' }) | |
| .notNull() | |
| .default(sql`(unixepoch())`), | |
| }) | |
| export const systemHealth = sqliteTable('system_health', { | |
| component: text('component').primaryKey(), | |
| status: text('status', { | |
| enum: ['healthy', 'degraded', 'down', 'unknown'], | |
| }) | |
| .notNull() | |
| .default('unknown'), | |
| message: text('message'), | |
| lastChecked: integer('last_checked', { mode: 'timestamp' }) | |
| .notNull() | |
| .default(sql`(unixepoch())`), | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/db/schema.ts` around lines 211 - 223, The systemHealth table as defined
(sqliteTable('system_health') with columns id, component, status, message,
lastChecked) lacks a uniqueness constraint on component so repeated health
checks will append rows; change the schema to enforce one-row-per-component by
making component the primary key (remove/stop using autoIncrement id) or add a
UNIQUE constraint on component and switch write code to use upsert/ON CONFLICT
to update lastChecked/status in the systemHealth table (refer to systemHealth,
component, id, lastChecked); alternatively, if you want audit behavior keep id
autoIncrement but add a retention policy/job to prune old rows and document the
intent.
🗄️ Database Naming Convention Updated (Commit
|
bea637d to
0292e32
Compare
✅ TypeScript Errors Fixed & CI Documentation Added (Commit
|
- Remove Prisma dependencies and generated files - Add Drizzle ORM with better-sqlite3 - Create comprehensive schema with 11 tables: * Device settings & configuration * Side settings & tap gestures * Temperature/power/alarm schedules * Device state (runtime) * Biometrics (sleep, vitals, movement) * System health monitoring - Configure SQLite with WAL mode for concurrency - Add migration runner with auto-seeding - Update package.json scripts for Drizzle workflow feat(db): migrate from Prisma to Drizzle ORM - Remove Prisma dependencies and generated files - Add Drizzle ORM with better-sqlite3 - Create comprehensive schema with 11 tables: * Device settings & configuration * Side settings & tap gestures * Temperature/power/alarm schedules * Device state (runtime) * Biometrics (sleep, vitals, movement) * System health monitoring - Configure SQLite with WAL mode for concurrency - Add migration runner with auto-seeding - Update package.json scripts for Drizzle workflow - Replace ADR-0010 with Drizzle decision (no supersede)
Critical fixes: - Add proper database indexes using Drizzle's index() builder - Fix SideSelector ESLint violations (member delimiters, unused state) - Fix inconsistent stable trend icon in SideSelector - Reduce mmap_size to 256MB for embedded systems - Remove page_size pragma (no-op after DB creation) - Guard signal handlers against duplicate registration - Fix relative migrationsFolder path using import.meta.url - Wrap seed inserts in transaction for atomicity - Fix brace style for catch blocks - Remove misleading default for sideSettings.name Changes: - src/db/schema.ts: Convert standalone indexes to inline definitions - src/db/index.ts: Optimize pragmas and fix signal handlers - src/db/migrate.ts: Fix path resolution and add transaction - src/components/SideSelector/SideSelector.tsx: Fix ESLint issues - Generate new migration with proper CREATE INDEX statements
- Fix multiline-ternary and jsx-one-expression-per-line violations in SideSelector - Fix arrow-parens violations in schema index definitions - Add comprehensive PR review process documentation The documentation captures our process for handling review feedback: - Always review and categorize comments - Fix critical/major issues in PR - File GitHub issues for minor improvements - Respond inline to all comments - Document summary of changes
- Remove Prisma cache paths from GitHub Actions - Remove Prisma generate step from CI workflow - Update cache key to use Drizzle schema instead of Prisma - Remove Prisma packages from pnpm-workspace.yaml onlyBuiltDependencies This completes the migration from Prisma to Drizzle ORM.
- Replace Prisma comment with SQLite Database - Update database ignore pattern from specific path to *.db pattern - Add SQLite WAL files (*.db-shm, *.db-wal) to gitignore The new patterns properly ignore SQLite database files and their Write-Ahead Logging (WAL) support files.
- Explicitly ignore main database: sleepypod.db and its WAL files - Add patterns for test/dev databases (*.test.db, *.dev.db, *.local.db) - Remove broad *.db pattern in favor of specific naming conventions This approach is more explicit and similar to the previous Prisma setup, while avoiding accidentally ignoring legitimate .db files that might be used for fixtures or test data.
- Change default database from sleepypod.db to sleepypod.dev.db - Update .gitignore to explicitly list sleepypod.dev.db - Simplify patterns to use specific filenames instead of wildcards Benefits: - Clear distinction between dev/test/production databases - More explicit and predictable .gitignore patterns - Follows convention similar to previous Prisma setup Production deployments should set DATABASE_URL environment variable to point to the actual production database location.
The JSX namespace wasn't available, causing TypeScript errors. Let TypeScript infer the return type instead of explicit annotation. Also add CI_CHECKS.md documentation to ensure developers run the same checks locally that GitHub Actions runs: - pnpm lint - pnpm tsc - pnpm test run --coverage --passWithNoTests This ensures PRs pass CI before pushing and saves review time.
c5c0c6a to
3fae491
Compare
🔄 Cleaned Up Commit HistoryRebased the branch to remove duplicate commits and ensure a clean linear history: Before
After
Final Commit ListThe merge blocking should now be resolved! Ready to merge 🚀 |
## Summary Adds a hardware abstraction layer for communication with the SleepyPod hardware control unit over TCP socket. ## What's Included ### Core Components **`src/hardware/client.ts`** - Main hardware client - High-level API for hardware operations - Connection management with auto-reconnect - Command queuing and response handling - Error handling and timeout management **`src/hardware/socketClient.ts`** - Low-level socket client - TCP socket connection wrapper - Binary protocol handling - Connection state management **`src/hardware/messageStream.ts`** - Message stream parser - Length-prefixed message parsing - Async message reading - Stream error handling **`src/hardware/responseParser.ts`** - Response parser - CBOR response decoding - Type validation - Error response handling **`src/hardware/sequentialQueue.ts`** - Command queue - Sequential command execution - Promise-based queuing - Prevents command overlap **`src/hardware/types.ts`** - Type definitions - Hardware command types - Response types - Configuration interfaces ## Changes from Previous Version - ✅ Rebased onto latest dev (includes Drizzle migration) - ✅ Fixed all ESLint violations - ✅ Removed forbidden non-null assertions - ✅ Added proper null checks - ✅ Fixed import type usage - ✅ All CI checks passing ## Dependencies Added - `binary-split` - Length-prefixed message parsing - `cbor-x` - CBOR encoding/decoding - `@types/binary-split` - TypeScript types ## Testing - ✅ Lint: passed - ✅ TypeScript: passed - ✅ Unit tests: passed (no tests yet) ## Next Steps After this merges: - Add unit tests for hardware layer - Add integration tests with mock hardware - Implement remaining hardware commands ## Related - Builds on PR #95 (Drizzle migration) - Required for tRPC routers (next PR) - Required for job scheduler (subsequent PR) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Hardware control library for pod device management * Temperature and power adjustment per side with validation * Alarm configuration and management * Device status monitoring with gesture detection * Automatic reconnection and sequential command execution for reliability * **Dependencies** * Added binary-split and cbor-x for hardware protocol support * **Documentation** * Added hardware integration documentation and testing guides <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Production-ready installation scripts for deploying SleepyPod to Eight Sleep Pod hardware, **comprehensively audited and hardened** by three expert agents for embedded device security and reliability. ## 🔍 Expert Security Audit Reviewed by 3 specialist agents: - **bash-script-expert** - Shell scripting, portability, error handling - **security-expert** - Vulnerabilities, supply chain, hardening - **devops-expert** - Reliability, deployment, production readiness **Original findings:** 56 issues **Context-adjusted:** 10 critical for jailbroken embedded device **Status:** ✅ All fixed [Full audit report](.claude/docs/installation-scripts-audit.md) ## 🎯 Critical Fixes Implemented ### 1. sp-update Completely Rewritten (Was Broken) **Problem:** Missing migrations, no rollback, would brick device on failed update **Fixed:** - ✅ Database backup before every update - ✅ Git rollback on failure - ✅ Database restore on failure - ✅ Stop service during update (prevent state mismatch) - ✅ Health check validation - ✅ Pre-flight checks (network, disk space) - ✅ Actually applies migrations (was missing) ### 2. Install Script Hardened - ✅ Pre-flight checks (500MB disk, network, dependencies) - ✅ Lock file prevents concurrent installs - ✅ Input validation (socket path existence) - ✅ Auto-detect network interface (not hardcoded wlan0) - ✅ Preserve .env on re-runs - ✅ Safe migrations (not destructive db:push) - ✅ Portable grep (BusyBox compatible) ### 3. npm Supply Chain Protection - ✅ `pnpm install --ignore-scripts` (prevent arbitrary code execution as root) - ✅ Explicitly build better-sqlite3 only - ✅ `--frozen-lockfile` for reproducible builds ### 4. Error Handling - ✅ `set -euo pipefail` (catch pipe failures) - ✅ Trap handlers with cleanup - ✅ Rollback on failure - ✅ Error messages to stderr ### 5. SSH Hardened - ✅ Keys-only authentication - ✅ Password auth disabled - ✅ SSH key format validation - ✅ Restore config on failure ### 6. IPv6 Firewall Gap Fixed - ✅ ip6tables rules (prevent bypass) - ✅ Custom chains (don't flush all rules) - ✅ Connection tracking - ✅ mDNS support ### 7. Script Standards - ✅ Removed .sh extensions - ✅ All scripts executable (chmod +x) - ✅ Updated documentation ## 🛡️ Context-Appropriate Security **What we kept (expert consensus):** - ✅ Service runs as root (required for dac.sock hardware control) - ✅ Minimal systemd hardening (avoid breaking jailbroken device) - ✅ No HTTPS/TLS (local network only, cert management = bricking risk) **Rationale:** This is a jailbroken embedded device with: - Single user (owner with root access) - Local network only (no internet exposure) - Hardware socket requires root - No physical access after deployment ## 📦 Scripts Included ### `scripts/install` Automated one-line installation with comprehensive safety checks: ```bash curl -fsSL https://raw.githubusercontent.com/sleepypod/core/main/scripts/install | sudo bash ``` **Features:** - Auto-detects dac.sock location - Installs Node.js 20 and pnpm - Clones and builds application - Initializes database with migrations - Creates systemd service - Creates CLI shortcuts - Pre-flight checks prevent bricking ### `scripts/sp-update` Embedded CLI tool with full rollback capability (embedded in install script): ```bash sp-update ``` **Features:** - Database backup before update - Git rollback on failure - Health check validation - Pre-flight checks - Service stop during update ### `scripts/setup-ssh` Configure SSH on port 8822 with keys-only authentication: ```bash sudo bash scripts/setup-ssh ``` **Features:** - Keys-only auth (passwords disabled) - SSH key format validation - Config backup and restore on failure ### `scripts/block-internet` Block all internet except local network (IPv4 + IPv6): ```bash sudo bash scripts/block-internet ``` **Features:** - IPv6 support (prevents bypass) - Custom iptables chains - mDNS support for local discovery - Connection tracking ### `scripts/unblock-internet` Restore internet access: ```bash sudo bash scripts/unblock-internet ``` ## 🚀 After Installation **CLI Commands:** - `sp-status` - View service status - `sp-restart` - Restart SleepyPod service - `sp-logs` - View live logs - `sp-update` - Update to latest version (with rollback) **Files:** - Database: `/persistent/sleepypod-data/sleepypod.db` - Config: `/home/dac/sleepypod-core/.env` - Service: `/etc/systemd/system/sleepypod.service` ## ✅ Testing - ✅ All scripts pass bash syntax validation - ✅ Scripts are executable - ✅ Documentation updated - ✅ Audit report in `.claude/docs/` ## 📊 Changes Summary **Files modified:** 10 **Lines changed:** +769, -322 **Scripts rewritten:** 2 (install, sp-update) **Scripts hardened:** 3 (setup-ssh, block-internet, unblock-internet) ## 🔗 Dependencies Requires recently merged PRs: - #95 - Database setup with Drizzle ORM - #100 - Hardware abstraction layer - #106 - tRPC routers - #111 - Job scheduler system --- 🤖 Generated with [Claude Sonnet 4.5](https://claude.com/claude-code) Reviewed by 3 expert agents: bash-script-expert, security-expert, devops-expert <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added a comprehensive installation guide covering prerequisites, one‑step install, post‑install commands, troubleshooting, and update procedures. * Added an installation audit summarizing reviews, risks, fixes, testing checklist, and deployment considerations. * **New Features** * Added an automated, resilient installation workflow with pre‑flight checks and safe rollback behavior. * Added a network access control tool to block or restore outbound connectivity with clear status and restore commands. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
🎉 This PR is included in version 1.1.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
Replaces Prisma with Drizzle ORM for better TypeScript integration and embedded system optimization.
Changes
Database Schema
11 tables with proper indexes:
Key Improvements
Testing
ADR
See
docs/adr/0010-drizzle-orm-sqlite.mdfor full rationale.Related
Part of sleepypod-core rewrite focusing on maintainability, type safety, and decoupled logic.
Summary by CodeRabbit
New Features
Documentation
Chores