feat(store): add MySQL record resolution - #79
Merged
Conversation
|
Warning Review limit reached
Next review available in: 29 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 (10)
📝 WalkthroughWalkthroughThe PR adds MySQL Record builders, validation, codecs, immutable resolution assets, and the synchronous ChangesSQL Record resolution
Estimated code review effort: 5 (Critical) | ~90+ minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant RecordDefinitions
participant resolveMysqlRecords
participant MysqlColumnResolver
participant MysqlRecordResolution
RecordDefinitions->>resolveMysqlRecords: provide records and overrides
resolveMysqlRecords->>MysqlColumnResolver: resolve types, defaults, and generated metadata
MysqlColumnResolver->>MysqlRecordResolution: create immutable tables, columns, references, and codecs
MysqlRecordResolution-->>RecordDefinitions: return resolved MySQL Records
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: 11
🤖 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/store/src/sql/mysql/adapter.ts`:
- Around line 59-82: Extract sqlOpaqueFormatSymbol, the shared predicates
isRecordContainer, hasStatementStructure, and isValidEnumValue, plus the table,
column, and SQL column-type allowlists, into one package-internal module. Remove
the duplicate declarations from the MySQL adapter and record.ts, import the
shared symbols in both files, and preserve identical predicate behavior and
allowlist contents for authoring and resolution checks.
- Around line 746-756: Update packages/store/src/sql/mysql/adapter.ts:746-756 in
decimalFits to reject decimal values whose parts.fraction.length exceeds scale,
preserving the existing precision and unsigned checks. Also update
packages/store/src/sql/mysql/adapter.ts:685-709 in the encode path to validate
fractional seconds through the fsp-aware normalizer, so values beyond the
resolved fsp are rejected before writing.
- Around line 643-667: Update the hour capture in parseTime to accept one to
three digits instead of requiring exactly three, while preserving the existing
minute, second, fraction, range, and negative-zero validation. Keep
normalizeTime responsible for producing the canonical three-digit hour
representation.
- Around line 711-727: Update binaryCodec’s encode function to pad decoded
base64 bytes with 0x00 up to the declared length before returning them, matching
the existing char branch’s MySQL padding behavior while preserving rejection of
payloads exceeding length.
- Around line 2052-2086: Update the table collision key in the surrounding
table-name validation logic to apply the same locale-independent folding used by
foldMysqlDatabaseName to tableName before composing tableKey. Preserve database
qualification and duplicate-name reporting, and add coverage proving differently
cased table names in the same database conflict.
In `@packages/store/src/sql/mysql/mysql-type-options.ts`:
- Around line 30-40: Update isMysqlFloatPrecisionOption so undefined scale is
accepted only when type is "float"; reject undefined scale for "double" and
"real" while preserving the existing precision range checks and no-precision
behavior.
In `@packages/store/test/sql/mysql/record.test.ts`:
- Around line 303-305: Extend the integer column tests around
table.columns.integer.encode/decode to assert the safe upper boundary:
Number.MAX_SAFE_INTEGER must be accepted, while Number.MAX_SAFE_INTEGER + 2 must
be rejected according to the existing encode/decode validation policy. Preserve
the current normal-value and string-type assertions.
- Around line 246-249: Update the MySQL type assertions in the test around
mysql.enum and mysql.bigint to use Vitest’s supported toExtend matcher instead
of deprecated toMatchTypeOf, preserving the existing expected enum and JobId
subtype relationships.
- Around line 411-415: Add an assertion in the custom encoder test that invokes
resolution.tables.custom.columns.value.encode({ x: -1 }) and verifies it throws
the expected “encode failed” error, placing it before the existing call-count
assertion. Preserve the current encoder behavior and call-count checks.
- Around line 314-342: Extend the direct MySQL helper boundary tests for
mysql.smallint and mysql.mediumint alongside the existing tinyint and int cases.
Assert successful encoding at each type’s signed maximum (16-bit and 24-bit) and
assert one-beyond-maximum values overflow, using the existing integer
boundary-test structure and symbols.
- Around line 595-623: The test assertions around the two autoDefault entries
must distinguish their independent validation failures. Update the failure
assertions to verify the corresponding messages for each autoDefault issue,
ensuring both the nullable autoIncrement rule and the autoIncrement-with-default
rule are independently covered rather than relying only on identical codes and
paths.
🪄 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: 7c9b1b14-a9e6-4526-8441-9f5b790e3ac4
📒 Files selected for processing (8)
.changeset/warm-jobs-resolve.mdpackages/store/README.mdpackages/store/package.jsonpackages/store/src/sql/mysql/adapter.tspackages/store/src/sql/mysql/index.tspackages/store/src/sql/mysql/mysql-type-options.tspackages/store/src/sql/mysql/record.tspackages/store/test/sql/mysql/record.test.ts
Contributor
Author
|
Follow-up refactor in
Verification:
|
There was a problem hiding this comment.
Actionable comments posted: 17
🤖 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/store/src/sql/mysql/column-codecs.ts`:
- Around line 302-340: Update decimalFits in
packages/store/src/sql/mysql/column-codecs.ts:302-340 to reject values whose
fraction length exceeds scale, matching normalizeDecimal and ensuring
decimalCodec.encode enforces precision consistently. In
packages/store/src/sql/mysql/column-codecs.ts:241-265, derive validate from the
fsp normalizer so encode rejects excess fractional digits for datetime,
timestamp, and time codecs.
In `@packages/store/src/sql/mysql/metadata.ts`:
- Around line 71-83: Reorder the validation in validStatement so the
fragments.some parameter check runs before the
fragments.length/hasMysqlStatementStructure check. Preserve the existing
parameter error and structure validation, ensuring statements containing SQL
parameters report “must not contain SQL parameters” instead of the
nonempty-structure error.
In `@packages/store/src/sql/mysql/record-resolver.ts`:
- Around line 467-481: Update duplicate detection in the columnNameAssets loop
to key seenColumnNames by the MySQL-folded form of asset.name, while retaining
asset.name’s original spelling in the issue message. Reuse the existing
database/table name-folding behavior or helper used elsewhere in this resolver
so names differing only by case are treated as duplicates.
In `@packages/store/src/sql/postgres/column-codecs.ts`:
- Around line 292-294: Update validUuid to validate only the canonical UUID
hexadecimal shape and separators, removing the version and variant nibble
restrictions. Preserve validation for malformed values while allowing
PostgreSQL-supported nil, max, and non-RFC variant UUIDs through encoding and
decoding.
- Around line 270-284: Document in the PostgreSQL adapter contract, alongside
the MySQL UTC contract, that temporal date, time, and timestamp OIDs must use
text parsers when the driver returns objects such as Date instances. Reference
temporalCodec’s string-only decode behavior and clearly state that adapters must
register these parsers.
- Around line 300-314: Update numericCodec’s valid function to inspect the
fractional portion of the unsigned value and reject values whose fractional
digit count exceeds the configured scale. Preserve the existing precision
validation and behavior when scale is not configured, while ensuring numeric
values such as 1.2345 are rejected for numeric(10, 2).
- Around line 381-389: Update the "char" and "varchar" branches in the codec
resolver to reuse the validated length option from directResolved/options and
reject encoded strings whose Unicode code-point count exceeds it with the
established codec TypeError behavior. Preserve trailing-space handling for
"char", and count characters rather than UTF-8 bytes.
- Around line 129-151: Update parsePostgresDate to reject year zero and enforce
PostgreSQL’s supported date range of 4713-01-01 BC through 5874897-12-31,
including the relevant month/day boundaries. Keep valid in-range dates unchanged
while returning undefined for zero, out-of-range, and oversized years so
validDate inherits the same validation.
In `@packages/store/src/sql/postgres/column-type-resolver.ts`:
- Around line 301-308: Update the invalidValue calls in encodeValue and
decodeValue to pass grammatical type tokens, so the formatted PostgreSQL codec
error reads correctly for both custom encoder and decoder outputs.
- Around line 235-253: Update the PostgreSQL array handling in
resolvePhysicalType so element formats with the portable dialect are normalized
through portableType before validation, matching the top-level portable-type
contract. Preserve PostgreSQL element handling and the existing invalid-element
issue for unsupported formats; do not reject valid portable scalar elements
solely because they are not already dialect-specific.
In `@packages/store/src/sql/postgres/record-resolver.ts`:
- Around line 99-110: Update resolveIdentity’s sequence-option handling to
reject unknown keys instead of silently ignoring them, matching
validateDirectOptions. Validate the sequence record’s keys against the supported
options (name, startWith, incrementBy, minValue, maxValue, cache, and cycle),
append an invalid-database-options issue for any unknown key, and preserve the
existing resolution behavior for valid keys.
- Around line 276-315: Remove the raw postgres metadata pre-validation loop in
packages/store/src/sql/postgres/record-resolver.ts lines 276-315; the effective
values are already validated by tableNameValid and schemaValid. Also remove the
raw column.postgres.name validation block at lines 326-337; columnName
validation at lines 344-354 is sufficient. These are the only affected sites.
- Around line 382-395: Update the notNull override handling in the record
resolver to treat null from ownNullableOverride as an absent, cleared override:
exclude null from the invalid-option check and let the derived
selectedPresence/physical nullability determine notNull. Preserve explicit
boolean overrides and the existing explicitNotNull === false identity-conflict
behavior.
- Around line 134-149: Update the sequence-options loop around
normalizeExactInteger so cache is normalized without applying the column range;
retain its existing cache validation elsewhere. When normalization returns
undefined, report an invalid/non-integer option instead of the column-range
error, and reserve the range message for integer values outside range.
In `@packages/store/src/sql/postgres/resolution-types.ts`:
- Around line 35-39: Add a type guard in the arrayCodec handling path that
validates the presence and expected shapes of values and lowerBounds, and
narrows the result to PostgresArrayDriverValue. Replace the separate
Reflect.get/Object.hasOwn checks with this guard, importing and using the
exported interface so runtime validation remains tied to the public contract.
In `@packages/store/src/sql/record-catalog-resolver.ts`:
- Around line 171-210: Update hasNestedOwnValue to return false when the path is
empty and the supplied value is undefined, while preserving true for an existing
candidate with an empty path and the current nested-property traversal behavior.
This ensures sqlDefinitionSourcePath only selects an overrides path when the
candidate actually exists.
In `@packages/store/test/sql/mysql/record.test.ts`:
- Around line 306-309: Update the invalid integer assertions in the integer
encode/decode tests to use Number.MAX_SAFE_INTEGER + 1 instead of + 2, ensuring
both methods reject the first unsafe integer value while preserving the existing
TypeError expectations.
🪄 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: 0eed0c63-319c-4016-8404-a3e0b8080c22
📒 Files selected for processing (18)
packages/store/src/sql/mysql/adapter.tspackages/store/src/sql/mysql/column-codecs.tspackages/store/src/sql/mysql/column-resolution.tspackages/store/src/sql/mysql/column-type-resolver.tspackages/store/src/sql/mysql/metadata.tspackages/store/src/sql/mysql/mysql-contract.tspackages/store/src/sql/mysql/name-folding.tspackages/store/src/sql/mysql/record-resolver.tspackages/store/src/sql/mysql/record.tspackages/store/src/sql/mysql/resolution-types.tspackages/store/src/sql/postgres/adapter.tspackages/store/src/sql/postgres/column-codecs.tspackages/store/src/sql/postgres/column-type-resolver.tspackages/store/src/sql/postgres/metadata.tspackages/store/src/sql/postgres/record-resolver.tspackages/store/src/sql/postgres/resolution-types.tspackages/store/src/sql/record-catalog-resolver.tspackages/store/test/sql/mysql/record.test.ts
This was referenced Aug 10, 2026
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
Verification
pnpm run verifypnpm --filter @commissary/store testpnpm --filter @commissary/store typecheckpnpm --filter @commissary/store buildpnpm --filter @commissary/store pack:checkCloses #61
Summary by CodeRabbit
New Features
Documentation
Tests