Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR changes ID/sequence handling across the codebase: OpenAPI3 and Swagger2 formatters now map the Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
Security Scan Results for PRDocker Image Scan Results
Source Code Scan Results🎉 No vulnerabilities found! |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/Appwrite/Utopia/Response/Model/Document.php (1)
39-45:⚠️ Potential issue | 🟡 MinorSame
default/examplestring-vs-integer mismatch as inRow.php.
'default' => ''and'example' => '1'should be0and1(integers) respectively to match theTYPE_ID → integermapping and the runtime cast introduced on line 87.🛠️ Proposed fix
->addRule('$sequence', [ 'type' => self::TYPE_ID, 'description' => 'Document sequence ID.', - 'default' => '', - 'example' => '1', + 'default' => 0, + 'example' => 1, 'readOnly' => true, ])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Appwrite/Utopia/Response/Model/Document.php` around lines 39 - 45, The $sequence rule in the Document model uses TYPE_ID (an integer type) but sets 'default' => '' and 'example' => '1', causing a string-vs-integer mismatch; update the addRule entry for '$sequence' in the Document class to use integer values ('default' => 0 and 'example' => 1) so they match TYPE_ID and the runtime cast logic that handles sequence IDs.src/Appwrite/Utopia/Response/Model/Row.php (1)
39-45:⚠️ Potential issue | 🟡 Minor
defaultandexamplevalues are strings but the field is now integer-typed.
TYPE_IDnow maps tointegerin both OpenAPI3 and Swagger2 specs, and thefilter()method casts$sequencetoint. The'default' => ''and'example' => '1'(string) values are inconsistent with the actual integer type, which can cause SDK generators to emit incorrect example/default types.🛠️ Proposed fix
->addRule('$sequence', [ 'type' => self::TYPE_ID, 'description' => 'Row sequence ID.', - 'default' => '', - 'example' => '1', + 'default' => 0, + 'example' => 1, 'readOnly' => true, ])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Appwrite/Utopia/Response/Model/Row.php` around lines 39 - 45, The default and example for the '$sequence' rule are strings but the field is integer-typed (TYPE_ID) and cast to int in the filter() method; update the addRule entry for '$sequence' to use integer values (e.g., 'default' => 0 and 'example' => 1) so OpenAPI/Swagger generators see numeric types and examples consistent with TYPE_ID and the filter() behavior, leaving 'readOnly' and other keys unchanged.
♻️ Duplicate comments (1)
src/Appwrite/SDK/Specification/Format/OpenAPI3.php (1)
845-847: Breaking-change concern already flagged inSwagger2.phpapplies here equally.
TYPE_ID→integerin the OpenAPI3 component schemas is the same breaking contract change. Marking to avoid a duplicate comment here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Appwrite/SDK/Specification/Format/OpenAPI3.php` around lines 845 - 847, The switch branch mapping TYPE_ID (the case 'id' in OpenAPI3.php) currently returns 'integer', which duplicates the breaking contract change flagged in Swagger2; change the mapping back to the non-breaking type (e.g., 'string' or the previously used representation for TYPE_ID) so component schemas keep the original contract. Update the case 'id' branch in the OpenAPI3 specification generator (where TYPE_ID is handled) to emit the original type/format used before the breaking change and ensure any related schema generation logic references the same representation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Appwrite/SDK/Specification/Format/OpenAPI3.php`:
- Around line 845-847: The OpenAPI3.php switch handling the 'id' type sets $type
= 'integer' but leaves $format null, causing sequence schemas to lack a format;
update the 'id' case (in the same switch where $type and $format are set) to
also assign $format = $rule['format'] ?? 'int32' (mirroring the fix in
Swagger2.php) so that the integer schema for $sequence includes the proper
format.
In `@src/Appwrite/SDK/Specification/Format/Swagger2.php`:
- Around line 825-827: Update the 'id' switch case in Swagger2.php to explicitly
set $format (same pattern as the 'integer' case) so ID fields emit a format;
specifically assign $format = $rule['format'] ?? 'int32' (or 'int64' if your IDs
can exceed 2,147,483,647) next to where $type = 'integer' is set, ensuring the
variables $type, $format and $rule are used consistently when serializing the
schema.
---
Outside diff comments:
In `@src/Appwrite/Utopia/Response/Model/Document.php`:
- Around line 39-45: The $sequence rule in the Document model uses TYPE_ID (an
integer type) but sets 'default' => '' and 'example' => '1', causing a
string-vs-integer mismatch; update the addRule entry for '$sequence' in the
Document class to use integer values ('default' => 0 and 'example' => 1) so they
match TYPE_ID and the runtime cast logic that handles sequence IDs.
In `@src/Appwrite/Utopia/Response/Model/Row.php`:
- Around line 39-45: The default and example for the '$sequence' rule are
strings but the field is integer-typed (TYPE_ID) and cast to int in the filter()
method; update the addRule entry for '$sequence' to use integer values (e.g.,
'default' => 0 and 'example' => 1) so OpenAPI/Swagger generators see numeric
types and examples consistent with TYPE_ID and the filter() behavior, leaving
'readOnly' and other keys unchanged.
---
Duplicate comments:
In `@src/Appwrite/SDK/Specification/Format/OpenAPI3.php`:
- Around line 845-847: The switch branch mapping TYPE_ID (the case 'id' in
OpenAPI3.php) currently returns 'integer', which duplicates the breaking
contract change flagged in Swagger2; change the mapping back to the non-breaking
type (e.g., 'string' or the previously used representation for TYPE_ID) so
component schemas keep the original contract. Update the case 'id' branch in the
OpenAPI3 specification generator (where TYPE_ID is handled) to emit the original
type/format used before the breaking change and ensure any related schema
generation logic references the same representation.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
src/Appwrite/SDK/Specification/Format/OpenAPI3.phpsrc/Appwrite/SDK/Specification/Format/Swagger2.phpsrc/Appwrite/Utopia/Response/Model/Document.phpsrc/Appwrite/Utopia/Response/Model/Row.php
🔄 PHP-Retry SummaryFlaky tests detected across commits: Commit
|
| Test | Retries | Total Time | Details |
|---|---|---|---|
UsageTest::testDatabaseStatsCollectionsAPI |
1 | 10.21s | Logs |
FunctionsCustomServerTest::testCreateDeploymentFromCLI |
1 | 407ms | Logs |
Commit a43e949 - 2 flaky tests
| Test | Retries | Total Time | Details |
|---|---|---|---|
UsageTest::testDatabaseStatsCollectionsAPI |
1 | 10.20s | Logs |
LegacyCustomServerTest::testOperators |
1 | 240.61s | Logs |
Commit 7dc50b4 - 8 flaky tests
| Test | Retries | Total Time | Details |
|---|---|---|---|
UsageTest::testDatabaseStatsCollectionsAPI |
1 | 10.18s | Logs |
LegacyConsoleClientTest::testBulkOperators |
1 | 240.33s | Logs |
LegacyCustomClientTest::testUpsertDocument |
1 | 240.97s | Logs |
LegacyCustomServerTest::testSpatialRelationshipManyToOne |
1 | 240.19s | Logs |
LegacyPermissionsGuestTest::testWriteDocument |
1 | 240.39s | Logs |
TablesDBConsoleClientTest::testNotEndsWith |
1 | 240.64s | Logs |
TablesDBCustomServerTest::testSpatialColCreateOnExistingData |
1 | 240.45s | Logs |
LegacyTransactionsConsoleClientTest::testCreateOperations |
1 | 240.71s | Logs |
✨ Benchmark results
⚡ Benchmark Comparison
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Appwrite/SDK/Specification/Format/OpenAPI3.php`:
- Around line 845-848: The OpenAPI3 specification generator currently sets IDs
to integer with a default format of 'int32' in the case 'id' branch; update that
fallback (in the switch handling for 'id' inside
src/Appwrite/SDK/Specification/Format/OpenAPI3.php) to use 'int64' instead of
'int32' so sequence/auto-increment IDs are represented as 64-bit integers (i.e.,
change the $rule['format'] ?? 'int32' fallback to $rule['format'] ?? 'int64').
In `@src/Appwrite/Utopia/Response/Model/Document.php`:
- Around line 86-87: The code is setting a default $sequence=0 via
getAttribute('$sequence', 0) which then gets materialized for documents that
never had the field; change the check to only act when the attribute actually
exists (e.g. use $document->hasAttribute('$sequence') or call
getAttribute('$sequence') without a default and check !== null), then if the
attribute exists and is numeric cast and call
$document->setAttribute('$sequence', (int)...). Update the block around the
Document handling (the $document variable / methods getAttribute, hasAttribute,
setAttribute) to avoid using the default 0 when determining whether to set the
attribute.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/Appwrite/Platform/Modules/Console/Http/Variables/Get.phpsrc/Appwrite/SDK/Specification/Format/OpenAPI3.phpsrc/Appwrite/SDK/Specification/Format/Swagger2.phpsrc/Appwrite/Utopia/Response/Model/ConsoleVariables.phpsrc/Appwrite/Utopia/Response/Model/Document.phpsrc/Appwrite/Utopia/Response/Model/Row.phptests/e2e/Scopes/Scope.phptests/e2e/Services/Databases/DatabasesBase.php
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Appwrite/Utopia/Response/Model/Row.php
- src/Appwrite/SDK/Specification/Format/Swagger2.php
What does this PR do?
(Provide a description of what this PR does and why it's needed.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Screenshots may also be helpful.)
Related PRs and Issues
Checklist