Fix: BigInt validator coercing non-validator attributes (#423)

[mwillbanks]

Fixes #423

The BigInt validator was attempting to coerce any value regardless if it was a validation attribute to a BigInt, this caused issues for things like @map(column_name) from working because it would throw an error attempting to coerce "column_name" into a BigInt value.

Changes:

• Stop coercing mapped attribute values into BigInt inside addBigIntValidation, limiting conversion to real validator attributes
• Add regression test proving @map-decorated BigInt fields create and update successfully

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Improved BigInt field validation in CRUD operations.

• Tests

  • Added regression test for BigInt ID handling in create and update operations.

[coderabbitai]

Walkthrough

Addresses issue #423 by refactoring the addBigIntValidation function to directly construct BigInt values without temporary variables, and adds a regression test verifying create and update operations with BigInt primary keys.

Changes

Cohort / File(s)	Summary
BigInt Validation Refactoring packages/orm/src/client/crud/validator/utils.ts	Eliminates temporary bigIntVal variable in addBigIntValidation, directly constructing BigInt(val) at each comparison operator branch (gt, gte, lt, lte) with identical semantics.
BigInt Regression Test tests/regression/test/issue-423.test.ts	New regression test file defining a SampleBigInt model with BigInt ID and String data field, verifying create and update operations resolve correctly with BigInt primary keys.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Verify the refactored BigInt construction maintains functional equivalence across all operator branches
• Confirm the regression test adequately covers the original issue #423 scenario with BigInt ID operations

Poem

🐰 With BigInts now dancing through validation's door,
No temp variables lurking anymore,
Direct and precise, each comparison clear,
The bug in regression shall never appear! ✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title clearly identifies the main issue being fixed: BigInt validator incorrectly coercing non-validator attributes, referenced with issue #423.
Linked Issues check	✅ Passed	The code changes directly address issue #423 by preventing BigInt validation from coercing non-validator attributes like @Map, and the regression test confirms create/update operations succeed.
Out of Scope Changes check	✅ Passed	All changes are narrowly scoped: refactoring BigInt validator logic in addBigIntValidation and adding a regression test for @map-decorated BigInt fields.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

• Provide your own instructions using the high_level_summary_instructions setting.
• Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
• Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
4. 📊 Contributor Summary — Include a Markdown table showing contributions:
   | Contributor | Lines Added | Lines Removed | Files Changed |
5. ✔️ Additional Notes — Add any extra reviewer context.
   Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull Request Overview

This PR fixes a bug where the BigInt validator was attempting to coerce all field attributes to BigInt values, including non-validation attributes like @map, causing errors when those attributes contained string values.

Key Changes:

• Modified addBigIntValidation to defer BigInt coercion until inside validation attribute matchers (@gt, @gte, @lt, @LTe)
• Added regression test validating that BigInt fields with @map decorator work correctly for create and update operations

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
packages/orm/src/client/crud/validator/utils.ts	Fixed premature BigInt coercion by moving conversion inside validation attribute matchers
tests/regression/test/issue-423.test.ts	Added regression test for BigInt fields with @Map decorator

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

packages/orm/src/client/crud/validator/utils.ts (1)

117-144: LGTM! Fix correctly prevents BigInt coercion of non-validator attributes.

The change successfully addresses issue #423 by moving the BigInt(val) conversion from an unconditional position (previously at line 128, before the match statement) into each validation branch. This ensures that only validator attributes (@gt, @gte, @lt, @lte) trigger BigInt conversion, while non-validator attributes like @map are safely ignored by the match statement.

The fix prevents the SyntaxError: Failed to parse String to BigInt that occurred when addBigIntValidation attempted to coerce @map("account_id") string values into BigInt.

Optional refinement for efficiency:

Currently, getArgValue is called for ALL attributes at line 124, even though only validation attributes need it. Consider checking attr.name first to avoid unnecessary value extraction for non-validator attributes:

 for (const attr of attributes) {
+    if (!['"@gt', '@gte', '@lt', '@lte'].includes(attr.name)) {
+        continue;
+    }
     const val = getArgValue<number>(attr.args?.[0]?.value);
     if (val === undefined) {
         continue;
     }

However, the current approach is consistent with addNumberValidation and addStringValidation, so this optimization is optional.

tests/regression/test/issue-423.test.ts (1)

4-16: Good regression test, but consider expanding coverage.

The test successfully verifies that BigInt fields with @map attributes no longer cause "Failed to parse String to BigInt" errors during create and update operations.

Enhance test coverage for validation attributes:

To ensure the fix doesn't break existing validation functionality, consider adding a test case that verifies BigInt validation attributes (@gt, @gte, @lt, @lte) still work correctly:

it('verifies BigInt validation attributes still work', async () => {
    const db = await createTestClient(
        `
model SampleBigInt {  
  id            BigInt       @id @map("sample_id")
  count         BigInt       @gt(0) @lte(1000)
}`
    );
    
    // Should succeed
    await expect(
        db.SampleBigInt.create({ data: { id: BigInt(1), count: BigInt(50) } })
    ).resolves.toMatchObject({ id: BigInt(1), count: BigInt(50) });
    
    // Should fail validation
    await expect(
        db.SampleBigInt.create({ data: { id: BigInt(2), count: BigInt(-1) } })
    ).rejects.toThrow();
});

Additionally, the original issue #423 mentioned @default(autoincrement()). Consider testing that scenario as well to ensure comprehensive coverage.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a831866 and edaabe8.

📒 Files selected for processing (2)

• packages/orm/src/client/crud/validator/utils.ts (1 hunks)
• tests/regression/test/issue-423.test.ts (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

tests/regression/test/issue-423.test.ts (1)

packages/testtools/src/client.ts (1)

• createTestClient (95-244)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Upload results

[ymc9]

LGTM! Thanks for making this PR. It took me a while to understand why it interferes with @map 😂