Conversation
kibertoad
requested review from
a team,
CarlosGamero and
dariacm
as code owners
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: lokalise/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis pull request extends migration support in the drizzle-utils module to handle both legacy (Drizzle-kit 0.x) and new (Drizzle-kit 1.0.0-beta) migration formats. The legacy format uses a flat SQL file structure with a Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/app/drizzle-utils/test/generateMigrations.ts (1)
99-106: Consider logging cleanup failures for debugging.Silent catch blocks can hide issues during local development. While acceptable for test utilities, logging (e.g.,
console.warn) could help diagnose flaky tests.Optional: Log cleanup failures
} finally { try { unlinkSync(schemaPath) - } catch {} + } catch (e) { + // Non-critical: file may not exist if generation failed early + } try { unlinkSync(configPath) - } catch {} + } catch (e) { + // Non-critical: file may not exist if generation failed early + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/drizzle-utils/test/generateMigrations.ts` around lines 99 - 106, In the finally block that cleans up files (the unlinkSync calls for schemaPath and configPath), replace the silent empty catch blocks with catches that log a warning including the error and which path failed (e.g., using console.warn with the error and the path variable); update the two catch blocks around unlinkSync(schemaPath) and unlinkSync(configPath) so they report failures instead of swallowing them to aid debugging of flaky tests.packages/app/drizzle-utils/src/markMigrationsApplied.ts (1)
292-295: Interpolation is safe here, but consider documenting the invariants.The comment correctly notes that
hash(hex string) andcreatedAt(number) are safe to interpolate. For defense-in-depth, you could add a validation check thathashmatches/^[0-9a-f]{64}$/before interpolation, though the current implementation is correct sincecomputeMigrationHashalways produces valid hex output.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/drizzle-utils/src/markMigrationsApplied.ts` around lines 292 - 295, Add a defensive validation before interpolating migration values: in the markMigrationsApplied logic where executor.run is called with tableName and entry.hash/entry.createdAt, verify entry.hash matches the expected hex invariant (e.g. /^[0-9a-f]{64}$/) and that entry.createdAt is a finite number; if validation fails, throw or log an error instead of interpolating. This keeps the current behavior (computeMigrationHash produces valid hex) but documents and enforces the invariant around the executor.run insertion using tableName, entry.hash, entry.createdAt and computeMigrationHash.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/app/drizzle-utils/src/markMigrationsApplied.ts`:
- Around line 292-295: Add a defensive validation before interpolating migration
values: in the markMigrationsApplied logic where executor.run is called with
tableName and entry.hash/entry.createdAt, verify entry.hash matches the expected
hex invariant (e.g. /^[0-9a-f]{64}$/) and that entry.createdAt is a finite
number; if validation fails, throw or log an error instead of interpolating.
This keeps the current behavior (computeMigrationHash produces valid hex) but
documents and enforces the invariant around the executor.run insertion using
tableName, entry.hash, entry.createdAt and computeMigrationHash.
In `@packages/app/drizzle-utils/test/generateMigrations.ts`:
- Around line 99-106: In the finally block that cleans up files (the unlinkSync
calls for schemaPath and configPath), replace the silent empty catch blocks with
catches that log a warning including the error and which path failed (e.g.,
using console.warn with the error and the path variable); update the two catch
blocks around unlinkSync(schemaPath) and unlinkSync(configPath) so they report
failures instead of swallowing them to aid debugging of flaky tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: lokalise/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b21de418-94d0-404d-b0ed-bedd1255fd8d
📒 Files selected for processing (19)
packages/app/drizzle-utils/README.mdpackages/app/drizzle-utils/docs/migrating-from-prisma.mdpackages/app/drizzle-utils/package.jsonpackages/app/drizzle-utils/src/index.tspackages/app/drizzle-utils/src/markMigrationsApplied.test.tspackages/app/drizzle-utils/src/markMigrationsApplied.tspackages/app/drizzle-utils/test/fixtures/migrations-new-format-cockroachdb/20260328163300_init/migration.sqlpackages/app/drizzle-utils/test/fixtures/migrations-new-format-cockroachdb/20260328163300_init/snapshot.jsonpackages/app/drizzle-utils/test/fixtures/migrations-new-format-cockroachdb/20260328163325_add_users/migration.sqlpackages/app/drizzle-utils/test/fixtures/migrations-new-format-cockroachdb/20260328163325_add_users/snapshot.jsonpackages/app/drizzle-utils/test/fixtures/migrations-new-format-mysql/20260328163343_init/migration.sqlpackages/app/drizzle-utils/test/fixtures/migrations-new-format-mysql/20260328163343_init/snapshot.jsonpackages/app/drizzle-utils/test/fixtures/migrations-new-format-mysql/20260328163417_add_users/migration.sqlpackages/app/drizzle-utils/test/fixtures/migrations-new-format-mysql/20260328163417_add_users/snapshot.jsonpackages/app/drizzle-utils/test/fixtures/migrations-new-format/20260328163300_init/migration.sqlpackages/app/drizzle-utils/test/fixtures/migrations-new-format/20260328163300_init/snapshot.jsonpackages/app/drizzle-utils/test/fixtures/migrations-new-format/20260328163325_add_users/migration.sqlpackages/app/drizzle-utils/test/fixtures/migrations-new-format/20260328163325_add_users/snapshot.jsonpackages/app/drizzle-utils/test/generateMigrations.ts
1 participant
Changes
Support drizzle snapshot format
Checklist
major,minor,patchorskip-releaseAI Assistance Tracking
We're running a metric to understand where AI assists our engineering work. Please select exactly one of the options below:
Mark "Yes" if AI helped in any part of this work, for example: generating code, refactoring, debugging support,
explaining something, reviewing an idea, or suggesting an approach.