removed check on primary

[ArnabChatterjee20k]

Summary by CodeRabbit

• Bug Fixes
  • Improved sequence validation to ensure comprehensive checking of all sequence types, strengthening database integrity verification.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6276e37e-0e1c-4fba-a62a-28393fe48bf3

📥 Commits

Reviewing files that changed from the base of the PR and between bb89e8c and daa6bac.

📒 Files selected for processing (1)

• src/Database/Validator/Sequence.php

💤 Files with no reviewable changes (1)

• src/Database/Validator/Sequence.php

---

📝 Walkthrough

Walkthrough

The change removes an early return statement in the sequence validator that was bypassing validation for non-primary key sequences. The validator now evaluates all sequences through the existing type-checking logic instead of returning prematurely.

Changes

Cohort / File(s)	Summary
Sequence Validation src/Database/Validator/Sequence.php	Removed early return that skipped validation for non-primary sequences; all sequences now proceed through the idAttributeType switch logic.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A guard that stood too tall, it seems,
Blocking paths through validation streams.
Now sequences flow without delay,
Each checked with care, come what may!
The rabbit hops with glee—validation's free! 🎉

🚥 Pre-merge checks | ✅ 3

✅ 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 'removed check on primary' directly describes the main change: removal of an early return that bypassed validation for non-primary sequences.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-tenant

📝 Coding Plan

• Generate coding plan for human review comments

---

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.}

[greptile-apps]

Greptile Summary

This PR removes the early-return guard (if (!$this->primary) { return true; }) from Sequence::isValid(), which previously exempted all non-$sequence VAR_ID attributes from UUID7 / integer-range format validation. After the change, every VAR_ID attribute — regardless of whether it is the primary $sequence field — must conform to the adapter's ID format.

Key concerns:

• Intentional regression of a prior fix: The removed guard was added deliberately in commit 05c5ff80 ("fix: skip format validation for non-primary VAR_ID attributes in Sequence") with a follow-up PHPStan fix in f5a989f6. Removing it without explaining why those fixes are no longer needed introduces uncertainty about whether edge cases they covered (e.g. MariaDB shared-table IDs, custom string $id values) still work correctly.
• Potentially breaking for existing callers: Any non-$sequence VAR_ID attribute value that was a valid string/int but not a UUID7 (MongoDB) or in-range integer (SQL) will now fail validation. This affects document creation/update validation (Structure.php) and query filter validation (Filter.php) alike.
• No tests accompany the change: The PR touches no test files. The behaviour being changed has direct coverage implications — the scenarios previously handled by the removed guard have no regression tests confirming the new rejection is intentional.
• default: return false in the switch now also applies to non-primary attributes; any future adapter that introduces a third idAttributeType would silently reject all non-$sequence VAR_ID values.

Confidence Score: 2/5

• Not safe to merge without clarification — this silently reverses a deliberate prior fix and ships no regression tests.
• The single removed block was added intentionally in a recent commit to fix a real issue (non-primary VAR_ID attributes bypassing format validation). Removing it without tests, without a documented reason, and without accounting for the default: return false path for potential future adapters introduces an unverified breaking change to both document validation and query filter validation paths.
• src/Database/Validator/Sequence.php — the only changed file; the removed guard has broad downstream impact through Structure.php and Filter.php.

Important Files Changed

Filename	Overview
src/Database/Validator/Sequence.php	Removes the early-return guard that skipped format validation for non-primary (non-$sequence) VAR_ID attributes, making UUID7/integer format validation apply uniformly to all VAR_ID attributes regardless of the $primary flag. This is a behaviour-changing, potentially breaking removal of a guard that was itself added as a deliberate fix in a recent commit.

Comments Outside Diff (2)

1. src/Database/Validator/Sequence.php, line 52-61 (link)

   Breaking change: strict format validation now applies to all VAR_ID attributes

   Before this PR the removed guard (if (!$this->primary) { return true; }) ensured that only the $sequence system attribute was subject to UUID7 / integer-range format validation. Every other VAR_ID attribute (e.g. $id, $internalId, or any user-defined foreign-key reference field) would accept any non-null string or integer and return true.

   With the guard gone, all VAR_ID attributes now fall through to the switch:

   • MongoDB (VAR_UUID7): any non-$sequence VAR_ID value must now match the UUID7 regex. Documents or query filters that carry a custom string $id (e.g. a human-readable slug, a UUIDv4, a Nano-ID) will now be rejected.
   • SQL (VAR_INTEGER): any non-$sequence VAR_ID value must now satisfy Range(1, MAX_BIG_INT). A string reference ID that was previously accepted would now return false.
   • default: return false – if a future adapter introduces a third idAttributeType, all non-$sequence VAR_ID values will silently fail validation.

   This guard was intentionally introduced in commit 05c5ff80 ("fix: skip format validation for non-primary VAR_ID attributes in Sequence") and the PHPStan follow-up f5a989f6. Removing it without a corresponding explanation of why that fix is no longer needed, and without regression tests for the scenarios it covered, is risky.

   Suggested verification / fix:

   If the intent truly is to enforce format validation everywhere, at minimum:

   1. Add a test that covers a non-primary VAR_ID attribute with a value that matched the old lenient path (any string/int) to confirm the new rejection is intentional.
   2. Consider whether default: return false should be default: return true (or throw) when an unknown idAttributeType is encountered for a non-primary attribute, so future adapters don't silently break.

2. src/Database/Validator/Sequence.php, line 38-50 (link)

   $primary property now serves only a single, narrow purpose

   After this change, $this->primary is only consulted in the empty($value) guard on line 40. Previously it also gated the entire format-validation block. Since the constructor still accepts and stores this flag, callers in Structure.php and Filter.php continue to pass it, but there is no indication in the code or the PR description that the reduced scope of $primary is intentional or that future uses are planned.

   If the flag is expected to do only the empty-string check, a brief code comment here would clarify the design intent and prevent a future developer from re-adding an if (!$this->primary) guard thinking it was accidentally omitted.

   _{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

_{Last reviewed commit: "removed check on pri..."}