feat(drizzle): add definition lifecycle - #85
Merged
Conversation
|
Warning Review limit reached
Next review available in: 46 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughChangesThe pull request adds Drizzle definition lifecycle
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant DrizzleStoreFactory
participant SharedDefinitionRuntime
participant DialectAdapter
participant DefinitionResult
Caller->>DrizzleStoreFactory: call define(...)
DrizzleStoreFactory->>SharedDefinitionRuntime: pass options and store kind
SharedDefinitionRuntime->>DialectAdapter: materialize tables and dialect assets
DialectAdapter-->>SharedDefinitionRuntime: return tables, references, and metadata
SharedDefinitionRuntime-->>DrizzleStoreFactory: return validated records and flat schema
DrizzleStoreFactory-->>Caller: return connection-free Store definition
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/drizzle/src/definition-runtime.ts`:
- Around line 1854-1857: Validate tableParts before constructing tableStatement,
accepting only one or two non-empty parts and throwing a clear error for empty,
overlong, or otherwise invalid input. Keep the existing one-part and two-part
SQL construction unchanged after validation.
- Around line 905-916: Split the combined condition in the generated-schema
validation around first, second, and structuralJsonEqual into separate
diagnostics: report when either parsed value is not JSON-compatible, and reserve
the existing “must produce a stable JSON value” diagnostic for two JSON values
that differ structurally. Preserve the current issue path and continuation
behavior for both cases.
- Around line 1619-1735: Add a lifecycle test for a thread-store definition that
triggers both host-record validation and Core composition failures. Assert the
complete ordered error.issues sequence, including the invalid-drizzle-override
issue at ["records"], and preserve the expected ordering of all host and Core
issues.
- Around line 842-866: Update the representative selection around
standardJsonSchemaInputRepresentative and structuralSchemaInputRepresentative to
detect tuple schemas and construct a non-empty candidate from their required
items before falling back to generic candidates. Ensure
z.tuple([z.literal("x")]) produces a valid representative and no longer reaches
the incompatible-generated-schema error, while preserving existing minLength
handling for long strings; add a static regression test covering this tuple
write schema.
- Around line 462-502: Add regression coverage for the write-schema fallback
using Zod 4.4.3, Zod 3.25.76, and Valibot 1.4.2, exercising each library’s
schema introspection path through structuralSchemaInputRepresentative. Add a
concise comment above structuralSchemaInputRepresentative documenting its
dependency on the version-sensitive Zod def/_def and Valibot pipe structures, so
dependency updates prompt review of these tests.
In `@packages/drizzle/src/definition-types.ts`:
- Around line 613-636: Replace the inline import types in both
DrizzleHooksConfig branches with the existing top-level ThreadRecordDefinitions
import, add RecordOverrides to the existing `@commissary/store` type import, and
use that imported symbol at the affected override references. Apply the same
import cleanup to the RecordDefinition reference in postgres.ts.
In `@packages/drizzle/src/drizzle-sql.ts`:
- Around line 18-23: Update the makePlaceholder callback passed to
compileSqlStatement in drizzleSql so it throws a clear error instead of
returning the literal "?". Preserve the existing never-parameter handling and
ensure any unexpected parameter fragment fails deterministically during SQL
definition.
In `@packages/drizzle/src/mysql-definition.ts`:
- Around line 53-56: Update mysqlTypeSql’s enum value serialization to escape
backslashes as well as single quotes when backslash escapes are enabled, using
the configured DDL SQL mode to avoid doubling them under NO_BACKSLASH_ESCAPES.
Preserve the existing quote escaping and enum formatting.
- Around line 183-204: Align codec installation with native builder selection
across all adapters: in packages/drizzle/src/mysql-definition.ts:183-204, extend
installMysqlAutoIncrementCodec to cover onUpdate === "current-timestamp"
columns; in packages/drizzle/src/postgres-definition.ts:147-154, verify whether
enum codecs are identity mappings and extend installPostgresIdentityCodec when
they are not; in packages/drizzle/src/sqlite-definition.ts:74-88, verify rowid
codec identity and add a matching installation pass when needed. Prefer deriving
installation targets directly from each dialect’s column-builder selection so
native-builder coverage cannot drift.
In `@packages/drizzle/src/postgres-definition.ts`:
- Around line 292-296: Update the asset-key mapping in the enum resolution flow
around enumValues to read schema and name directly from resolvedEnum,
eliminating the resolution.enums.find lookup and the unqualified entity.enumName
fallback; produce the same qualified key format as postgresEnumKey and
finishPostgresAssets.
- Around line 370-380: In the conflicting-definition branch of the materialized
enum collection logic, replace the second result entry with an
invalid-drizzle-enum diagnostic describing the differing enum value tuples and
identifying the affected enum or column path. Do not add the duplicate physical
key to result; preserve the existing behavior for identical definitions and
first-time keys.
In `@packages/drizzle/src/sqlite-definition.ts`:
- Around line 160-165: Extract the duplicated reverse column lookup into a
shared helper in definition-runtime.ts that accepts the columns map and
primary-key column list and returns logical field names. Update
getPrimaryKeyFields in packages/drizzle/src/sqlite-definition.ts#L160-L165,
packages/drizzle/src/mysql-definition.ts#L291-L296, and
packages/drizzle/src/postgres-definition.ts#L421-L426 to call the helper while
retaining each adapter’s dialect-specific config reading.
In `@packages/drizzle/test/definition-lifecycle.test.ts`:
- Around line 341-351: Update the remaining failure tests in
definition-lifecycle.test.ts to capture validation issues with
captureDrizzleIssueLocations instead of asserting only DrizzleDefinitionError.
For each referenced test, assert the exact issue codes, paths, and ordering
expected for the malformed or unsupported input, following the existing enum
tests as the pattern and preserving the non-JSON generator case’s expected
locations.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 50448bd2-f69a-4fe9-8d90-6afb0d7ee955
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
.changeset/tidy-hounds-spark.mdREADME.mdpackage.jsonpackages/drizzle/CONTEXT.mdpackages/drizzle/LICENSEpackages/drizzle/README.mdpackages/drizzle/package.jsonpackages/drizzle/src/definition-contracts.tspackages/drizzle/src/definition-runtime.tspackages/drizzle/src/definition-state.tspackages/drizzle/src/definition-types.tspackages/drizzle/src/drizzle-sql.tspackages/drizzle/src/index.tspackages/drizzle/src/mysql-definition.tspackages/drizzle/src/mysql.tspackages/drizzle/src/postgres-definition.tspackages/drizzle/src/postgres.tspackages/drizzle/src/sqlite-definition.tspackages/drizzle/src/sqlite.tspackages/drizzle/test/definition-lifecycle.test.tspackages/drizzle/tsconfig.build.jsonpackages/drizzle/tsconfig.jsonscripts/check-built-imports.mjsscripts/check-package-archive.mjsscripts/check-package-archive.test.mjsscripts/runtime-conformance-suite.mjs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/drizzle/test/definition-lifecycle.test.ts`:
- Around line 380-389: Refactor the unsupported schema-family test around
DrizzleSqliteStore.define to use captureDrizzleIssueLocations for a single
invocation and assertion. Remove the duplicate try/catch, DrizzleDefinitionError
instance check, and direct error.issues mapping while preserving the expected
unsupported-schema-family code and schemaGenerators path.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0cf2744d-818a-4f27-a85e-17dc9b75fbab
📒 Files selected for processing (15)
docs/specs/drizzle-mysql-store.mddocs/specs/drizzle-package-interface.mddocs/specs/drizzle-postgres-store.mddocs/specs/drizzle-store.mdpackages/drizzle/README.mdpackages/drizzle/src/definition-runtime.tspackages/drizzle/src/definition-types.tspackages/drizzle/src/drizzle-sql.tspackages/drizzle/src/mysql-definition.tspackages/drizzle/src/postgres-definition.tspackages/drizzle/src/postgres.tspackages/drizzle/test/definition-lifecycle.test.tspackages/store/prototypes/complete-sql-drizzle-specification.prototype.tspackages/store/prototypes/drizzle-package-interface.prototype.tspackages/store/prototypes/shared-drizzle-definition-lifecycle.prototype.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@commissary/drizzlepackage with connection-free PostgreSQL, MySQL, and SQLite Store and Thread Store definition factoriesCloses #64.
Verification
pnpm run verifypnpm --filter @commissary/drizzle typecheckpnpm --filter @commissary/drizzle testpnpm --filter @commissary/drizzle buildpnpm --filter @commissary/drizzle pack:checkpnpm run check:importspnpm run conformance:nodepnpm run conformance:bunpnpm run conformance:denopnpm run conformance:browserpnpm run conformance:cloudflareRelease
no-changesetlabel.No-Changeset reason, if applicable: N/A
Summary by CodeRabbit
New Features
@commissary/drizzlewith typed, connection-free Store and Thread Store definitions.Documentation
Updates
schemaGenerators.Release
@commissary/drizzle.