Skip to content

Conversation

@abnegate
Copy link
Member

@abnegate abnegate commented Feb 13, 2026

Summary

  • The $sequence attribute is always an auto-incrementing integer, regardless of the database adapter's ID type
  • When the Sequence validator checks $sequence values ($primary=true), it now always uses integer Range validation instead of applying the adapter's ID type validation
  • This fixes Query::equal('$sequence', [...]) returning 400 on MongoDB adapters where getIdAttributeType() returns VAR_UUID7

Test plan

  • Verify $sequence queries work on MongoDB adapter (previously returned 400)
  • Verify $sequence queries still work on SQL adapters (integer IDs)
  • Verify $id queries are unaffected (still use adapter-specific validation)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved validation for primary key sequences to enforce proper numeric ranges (1 to maximum limit).
    • Ensured consistent validation behavior for non-primary sequences with appropriate numeric range constraints.
    • Added clarification to sequence validation logic for improved code maintainability.

The $sequence attribute is always an auto-incrementing integer,
regardless of the database adapter's ID type. When the Sequence
validator checks $sequence values (primary=true), it should always
use integer Range validation instead of applying the adapter's
ID type validation (e.g., UUID7 regex for MongoDB).

This fixes Query::equal('$sequence', [...]) returning 400 on
MongoDB adapters where getIdAttributeType() returns VAR_UUID7.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 13, 2026

📝 Walkthrough

Walkthrough

The Sequence validator's isValid method was refactored to enforce numeric range validation for primary key sequences (1 to MAX_BIG_INT) using a Range validator, short-circuiting before type-based logic. Non-primary sequences consistently use Range(0, MAX_BIG_INT, VAR_INTEGER).

Changes

Cohort / File(s) Summary
Sequence Validator Refactoring
src/Database/Validator/Sequence.php
Reorganized validation logic to use Range validator for primary key sequences (1..MAX_BIG_INT), eliminating previous conditional start value logic. Non-primary sequences use consistent Range(0, MAX_BIG_INT, VAR_INTEGER) validation. Added clarifying comment on integer treatment for primary sequences.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Hop, hop! The sequence now leaps so clean,
Primary keys dance in ranges pristine,
From one to infinity's greatest sight,
The validator bounds them just right!

🚥 Pre-merge checks | ✅ 3 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: validating the $sequence field as an integer regardless of adapter type, which matches the core fix in the changeset.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/sequence-validator-integer

No actionable comments were generated in the recent review. 🎉


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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant