merge dev to main #154
merge dev to main #154
Conversation
Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com>
* chore: single better-sqlite version for all packages * update lock file --------- Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com>
…upBy` (#152) * fix: support using aggregations inside `orderBy` and `having` of `groupBy` * update * update
WalkthroughVersion bumps across the monorepo. Runtime gains aggregation-aware querying: new constants, typings, validators, query utils, and dialect logic; groupBy processing adjusted and post-processed. Minor wording change in language validator. Tests expanded for groupBy and aggregations. Some dependencies updated (better-sqlite3, @types/better-sqlite3). Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
Claude finished @ymc9's task —— View job Code Review AnalysisReviewing this PR that merges dev to main with aggregation support in Review Tasks:
Review SummaryThis is a substantial PR that adds aggregation support for Code Quality Assessment✅ Strengths
|
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades the project from version 3.0.0-alpha.19 to 3.0.0-alpha.20, with key improvements to the GroupBy functionality and dependency updates.
- Enhanced GroupBy operations with support for aggregations in having clauses and orderBy
- Updated better-sqlite3 dependencies to version 12.2.0
- Added comprehensive validation for aggregation operations in where clauses
Reviewed Changes
Copilot reviewed 29 out of 30 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Multiple package.json files | Version bump from 3.0.0-alpha.19 to 3.0.0-alpha.20 |
| packages/runtime/src/client/crud/validator.ts | Enhanced validation for aggregations in having clauses and primitive filters |
| packages/runtime/src/client/crud-types.ts | Added aggregation support to filter types and improved GroupBy type definitions |
| packages/runtime/test/client-api/group-by.test.ts | Comprehensive test coverage for new aggregation features |
| packages/runtime/src/client/crud/operations/group-by.ts | Improved GroupBy SQL generation with proper column references |
| packages/runtime/src/client/crud/dialects/base.ts | Added aggregation support in filters and orderBy operations |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (4)
packages/runtime/src/client/constants.ts (1)
26-31: Update PascalCase for AggregateOperator TypeTo align with TS conventions and prevent value/type shadowing, rename the type from
AGGREGATE_OPERATORStoAggregateOperatorand update all its imports/usages accordingly.Files to update:
- packages/runtime/src/client/constants.ts
• Change the type export.- packages/runtime/src/client/query-utils.ts
• ImportAggregateOperatorinstead ofAGGREGATE_OPERATORS.
• Update parameterop: AGGREGATE_OPERATORS→op: AggregateOperator.- packages/runtime/src/client/crud/validator.ts
• Import{ AGGREGATE_OPERATORS, … }→{ AGGREGATE_OPERATORS, … }(value) andtype { AggregateOperator }.
• ChangewithAggregations: Array<AGGREGATE_OPERATORS>→Array<AggregateOperator>.
• Update all casts and checks:
–key as AGGREGATE_OPERATORS→key as AggregateOperator- packages/runtime/src/client/crud/dialects/base.ts
• Importtype { AggregateOperator }in place of the type import.
• Update casts:(field as AGGREGATE_OPERATORS)→(field as AggregateOperator).Example diff for constants.ts:
export const AGGREGATE_OPERATORS = ['_count', '_sum', '_avg', '_min', '_max'] as const; -export type AGGREGATE_OPERATORS = (typeof AGGREGATE_OPERATORS)[number]; +export type AggregateOperator = (typeof AGGREGATE_OPERATORS)[number];packages/runtime/src/client/crud/dialects/base.ts (1)
737-749: Remove dead code: duplicate _count orderBy handling is unreachableThe earlier aggregation block handles all of ['_count','_avg','_sum','_min','_max'] and continues, so this special-case will never run. It’s confusing to keep around.
Apply this diff to remove the unreachable block:
- switch (field) { - case '_count': { - invariant(value && typeof value === 'object', 'invalid orderBy value for field "_count"'); - for (const [k, v] of Object.entries<string>(value)) { - invariant(v === 'asc' || v === 'desc', `invalid orderBy value for field "${field}"`); - result = result.orderBy( - (eb) => eb.fn.count(sql.ref(k)), - sql.raw(this.negateSort(v, negated)), - ); - } - continue; - } - default: - break; - } + // aggregate orderBy cases handled abovepackages/runtime/src/client/crud/validator.ts (1)
713-719: Validation gap: restrict _avg/_sum orderBy to numeric fieldsmakeOrderBySchema currently allows all scalar fields under _avg/_sum. This disagrees with typing (SumAvgInput) and will accept invalid payloads at runtime.
Apply this refactor to enforce numeric-only keys for _avg/_sum while reusing the existing schema for _count/_min/_max:
- if (WithAggregation) { - const aggregationFields = ['_count', '_avg', '_sum', '_min', '_max']; - for (const agg of aggregationFields) { - fields[agg] = z.lazy(() => this.makeOrderBySchema(model, true, false).optional()); - } - } + if (WithAggregation) { + const nonRelationOrderBy = z.lazy(() => this.makeOrderBySchema(model, true, false).optional()); + // unrestricted aggregations over all scalar fields + fields['_count'] = nonRelationOrderBy; + fields['_min'] = nonRelationOrderBy; + fields['_max'] = nonRelationOrderBy; + + // restrict _avg/_sum to numeric scalar fields + const modelDef = requireModel(this.schema, model); + const sort = z.union([z.literal('asc'), z.literal('desc')]); + const numericFields = Object.keys(modelDef.fields).filter((f) => { + const def = requireField(this.schema, model, f); + return !def.relation && !def.array && NUMERIC_FIELD_TYPES.includes(def.type); + }); + if (numericFields.length > 0) { + const numericOrderBy = z + .strictObject( + Object.fromEntries(numericFields.map((f) => [f, sort])) as Record<string, z.ZodType>, + ) + .optional(); + fields['_avg'] = numericOrderBy; + fields['_sum'] = numericOrderBy; + } + }packages/runtime/src/client/crud-types.ts (1)
236-239: Type bug: AND/OR/NOT drop WithAggregations in recursive WhereInputThe recursive WhereInput uses the default WithAggregations=false, even when the outer WhereInput sets WithAggregations=true (e.g., GroupByHaving). This prevents aggregations under logical combinators at the type level.
Apply this fix:
- AND?: OrArray<WhereInput<Schema, Model, ScalarOnly>>; - OR?: WhereInput<Schema, Model, ScalarOnly>[]; - NOT?: OrArray<WhereInput<Schema, Model, ScalarOnly>>; + AND?: OrArray<WhereInput<Schema, Model, ScalarOnly, WithAggregations>>; + OR?: WhereInput<Schema, Model, ScalarOnly, WithAggregations>[]; + NOT?: OrArray<WhereInput<Schema, Model, ScalarOnly, WithAggregations>>;
🧹 Nitpick comments (6)
packages/runtime/src/client/constants.ts (1)
21-25: Consider exporting a dedicated type alias for logical combinatorsFor readability and reuse in type signatures across the codebase, export a LogicalCombinator union type alongside the tuple.
Apply this diff:
export const LOGICAL_COMBINATORS = ['AND', 'OR', 'NOT'] as const; +export type LogicalCombinator = (typeof LOGICAL_COMBINATORS)[number];packages/runtime/src/client/crud/operations/group-by.ts (1)
47-50: Pass groupBy expressions via spread for broader Kysely compatibilitySome Kysely versions have overloads that accept multiple arguments or a single array. Using spread avoids relying on the array overload and makes the intent explicit.
Apply this diff:
-const bys = typeof parsedArgs.by === 'string' ? [parsedArgs.by] : (parsedArgs.by as string[]); -query = query.groupBy(bys.map((by) => sql.ref(`$sub.${by}`))); +const bys = typeof parsedArgs.by === 'string' ? [parsedArgs.by] : (parsedArgs.by as string[]); +query = query.groupBy(...bys.map((by) => sql.ref(`$sub.${by}`)));packages/runtime/test/client-api/group-by.test.ts (1)
222-262: Strengthen orderBy-by-aggregation assertion to check orderingCurrent assertion only checks presence. To verify ordering by _avg.age desc, assert the full ordered result.
Apply this diff:
- await expect( - client.profile.groupBy({ - by: ['bio'], - orderBy: { - _avg: { - age: 'desc', - }, - }, - }), - ).resolves.toEqual(expect.arrayContaining([{ bio: 'bio2' }])); + await expect( + client.profile.groupBy({ + by: ['bio'], + orderBy: { + _avg: { + age: 'desc', + }, + }, + }), + ).resolves.toEqual([{ bio: 'bio2' }, { bio: 'bio1' }]);packages/runtime/src/client/crud/dialects/base.ts (2)
724-735: OrderBy aggregation now unified via aggregate()This refactor reduces bespoke logic and centralizes aggregator handling. One follow-up: consider removing the now-redundant special-case below for _count to avoid confusion.
664-680: Minor readability nit: avoid shadowing “conditions”conditions is first an object, then spread as conditions.conditions. Consider renaming the variable to std or result to avoid confusion.
Apply this diff locally within the method:
- const conditions = this.buildStandardFilter( + const std = this.buildStandardFilter( eb, 'Bytes', payload, fieldRef, (value) => this.transformPrimitive(value, 'Bytes', false), (value) => this.buildBytesFilter(eb, fieldRef, value as BytesFilter<Schema, boolean, boolean>), true, ['equals', 'in', 'notIn', 'not'], ); - return this.and(eb, ...conditions.conditions); + return this.and(eb, ...std.conditions);packages/runtime/src/client/crud/validator.ts (1)
339-345: Intentional: negative take supportSwitching take to z.int() (not constrained to nonnegative) enables negative take semantics for reversed pagination. If that’s intended, consider a short doc comment to avoid confusion.
Would you like me to add a brief comment near makeTakeSchema explaining the rationale?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (29)
package.json(1 hunks)packages/cli/package.json(2 hunks)packages/common-helpers/package.json(1 hunks)packages/create-zenstack/package.json(1 hunks)packages/dialects/sql.js/package.json(1 hunks)packages/eslint-config/package.json(1 hunks)packages/ide/vscode/package.json(1 hunks)packages/language/package.json(1 hunks)packages/language/src/validators/datamodel-validator.ts(1 hunks)packages/runtime/package.json(2 hunks)packages/runtime/src/client/client-impl.ts(1 hunks)packages/runtime/src/client/constants.ts(1 hunks)packages/runtime/src/client/contract.ts(1 hunks)packages/runtime/src/client/crud-types.ts(8 hunks)packages/runtime/src/client/crud/dialects/base.ts(10 hunks)packages/runtime/src/client/crud/operations/group-by.ts(1 hunks)packages/runtime/src/client/crud/validator.ts(15 hunks)packages/runtime/src/client/query-utils.ts(2 hunks)packages/runtime/test/client-api/group-by.test.ts(6 hunks)packages/runtime/test/schemas/basic/schema.ts(1 hunks)packages/runtime/test/schemas/basic/schema.zmodel(1 hunks)packages/sdk/package.json(1 hunks)packages/tanstack-query/package.json(1 hunks)packages/testtools/package.json(2 hunks)packages/typescript-config/package.json(1 hunks)packages/vitest-config/package.json(1 hunks)packages/zod/package.json(1 hunks)samples/blog/package.json(2 hunks)tests/e2e/package.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/schema.zmodel
📄 CodeRabbit Inference Engine (CLAUDE.md)
Always run
zenstack generateafter modifying ZModel schemas
Files:
packages/runtime/test/schemas/basic/schema.zmodel
{packages,samples,tests}/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Packages are located in
packages/,samples/, andtests/
Files:
packages/runtime/test/schemas/basic/schema.zmodelpackages/language/src/validators/datamodel-validator.tspackages/language/package.jsonpackages/dialects/sql.js/package.jsonpackages/typescript-config/package.jsonpackages/runtime/package.jsonpackages/zod/package.jsonpackages/create-zenstack/package.jsonpackages/eslint-config/package.jsonsamples/blog/package.jsonpackages/sdk/package.jsonpackages/common-helpers/package.jsonpackages/vitest-config/package.jsontests/e2e/package.jsonpackages/cli/package.jsonpackages/runtime/test/schemas/basic/schema.tspackages/runtime/src/client/client-impl.tspackages/tanstack-query/package.jsonpackages/runtime/src/client/constants.tspackages/ide/vscode/package.jsonpackages/runtime/src/client/contract.tspackages/testtools/package.jsonpackages/runtime/src/client/crud/operations/group-by.tspackages/runtime/test/client-api/group-by.test.tspackages/runtime/src/client/crud/validator.tspackages/runtime/src/client/query-utils.tspackages/runtime/src/client/crud/dialects/base.tspackages/runtime/src/client/crud-types.ts
tests/e2e/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
E2E tests are in
tests/e2e/directory
Files:
tests/e2e/package.json
🧠 Learnings (2)
📚 Learning: 2025-08-04T08:43:33.161Z
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.161Z
Learning: `zenstack generate` compiles ZModel to TypeScript schema (`schema.ts`)
Applied to files:
packages/language/package.json
📚 Learning: 2025-08-04T08:43:33.161Z
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.161Z
Learning: No runtime dependency on prisma/client
Applied to files:
samples/blog/package.json
🧬 Code Graph Analysis (8)
packages/language/src/validators/datamodel-validator.ts (2)
packages/runtime/src/client/crud/dialects/postgresql.ts (1)
provider(27-29)packages/runtime/src/client/crud/dialects/sqlite.ts (1)
provider(27-29)
packages/runtime/src/client/client-impl.ts (1)
packages/runtime/src/client/crud/operations/group-by.ts (1)
GroupByOperationHandler(7-161)
packages/runtime/src/client/crud/operations/group-by.ts (1)
packages/runtime/src/helpers.ts (1)
sql(1-1)
packages/runtime/test/client-api/group-by.test.ts (1)
packages/runtime/test/client-api/utils.ts (1)
createPosts(23-32)
packages/runtime/src/client/crud/validator.ts (5)
packages/sdk/src/schema/schema.ts (3)
BuiltinType(84-94)EnumDef(98-98)GetModels(108-108)packages/language/src/generated/ast.ts (1)
BuiltinType(101-101)packages/runtime/src/utils/object-utils.ts (1)
extractFields(4-6)packages/runtime/src/client/constants.ts (3)
AGGREGATE_OPERATORS(29-29)AGGREGATE_OPERATORS(30-30)LOGICAL_COMBINATORS(24-24)packages/runtime/src/utils/enumerate.ts (1)
enumerate(9-17)
packages/runtime/src/client/query-utils.ts (2)
packages/sdk/src/schema/expression.ts (1)
Expression(1-10)packages/runtime/src/client/constants.ts (2)
AGGREGATE_OPERATORS(29-29)AGGREGATE_OPERATORS(30-30)
packages/runtime/src/client/crud/dialects/base.ts (3)
packages/runtime/src/client/constants.ts (3)
LOGICAL_COMBINATORS(24-24)AGGREGATE_OPERATORS(29-29)AGGREGATE_OPERATORS(30-30)packages/runtime/src/client/query-utils.ts (1)
aggregate(321-333)packages/runtime/src/client/crud-types.ts (4)
StringFilter(294-311)BooleanFilter(357-368)DateTimeFilter(331-340)BytesFilter(342-355)
packages/runtime/src/client/crud-types.ts (2)
packages/sdk/src/schema/schema.ts (9)
GetModelFieldType(151-155)ModelFieldIsOptional(217-221)SchemaDef(10-18)BuiltinType(84-94)GetModels(108-108)GetModelFields(132-135)FieldIsArray(235-239)FieldIsRelation(229-233)NonRelationFields(190-194)packages/runtime/src/utils/type-utils.ts (3)
NullableIf(5-5)OrArray(48-48)NonEmptyArray(50-50)
⏰ 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). (2)
- GitHub Check: build-test (20.x)
- GitHub Check: claude-review
🔇 Additional comments (35)
package.json (1)
3-3: Monorepo sync to 3.0.0-alpha.20 verified
No lingeringalpha.19references found; allpackage.jsonversions are at3.0.0-alpha.20.packages/zod/package.json (1)
3-3: LGTM: version bumpAligned with the monorepo release; no API surface changes in this package.json.
packages/common-helpers/package.json (1)
3-3: LGTM: version bumpConsistent with the coordinated alpha.20 release; no other changes.
packages/create-zenstack/package.json (1)
3-3: LGTM: version bumpMatches the repo-wide version; no functional changes.
packages/tanstack-query/package.json (1)
3-3: LGTM: version bumpIn sync with the monorepo; nothing else changed. Private package will be skipped by publish scripts as expected.
tests/e2e/package.json (2)
3-3: LGTM: version bump aligns with the monorepo release.
No functional impact; consistent with other packages moving to 3.0.0-alpha.20.
3-3: Monorepo version consistency confirmed
All package.json files share version 3.0.0-alpha.20, and every@zenstackhq/*dependency remains onworkspace:*. Ready for publishing.packages/eslint-config/package.json (1)
3-3: LGTM: metadata-only version bump.
Private package; no downstream risk. Matches the repo-wide alpha.20 release.packages/sdk/package.json (1)
3-3: LGTM: SDK package version increment only.
Exports, deps, and scripts unchanged; safe metadata update.packages/vitest-config/package.json (1)
4-4: LGTM: vitest-config version bump.
Config-only package; no functional changes introduced.packages/typescript-config/package.json (1)
3-3: LGTM: typescript-config version bump.
No behavioral impact; aligns with monorepo release.packages/dialects/sql.js/package.json (1)
3-3: Version bump looks goodRepo-wide alignment to 3.0.0-alpha.20 acknowledged. No API/manifest changes here.
samples/blog/package.json (2)
3-3: Sample app version bump is consistentVersion updated to 3.0.0-alpha.20, consistent with the workspace.
21-21: Consistent @types/better-sqlite3 versions confirmedRunning the provided script shows that all instances of
@types/better-sqlite3use^7.6.13:
- packages/cli/package.json:
^7.6.13- packages/runtime/package.json:
^7.6.13- samples/blog/package.json:
^7.6.13No action required.
packages/language/package.json (1)
4-4: Language package version bump is straightforwardNo functional or export changes here; just the version alignment.
packages/ide/vscode/package.json (1)
4-4: VS Code extension version updated — OKPre-release workflow remains the same; nothing else changed in the manifest.
packages/runtime/package.json (2)
3-3: Runtime package version bump aligns with repoNothing else changed in manifest contents. Good to go.
92-92: All better-sqlite3 versions are consistent across the workspaceAfter scanning every package.json that depends on better-sqlite3:
• packages/cli/package.json
– dependencies: better-sqlite3 ^12.2.0
– devDependencies: @types/better-sqlite3 ^7.6.13• packages/runtime/package.json
– dependencies: better-sqlite3 ^12.2.0
– devDependencies: @types/better-sqlite3 ^7.6.13• packages/testtools/package.json
– dependencies: better-sqlite3 ^12.2.0 (no types package)• samples/blog/package.json
– dependencies: better-sqlite3 ^12.2.0
– dependencies: @types/better-sqlite3 ^7.6.13All packages that declare better-sqlite3 use the same ^12.2.0 range, and all that include the types package use ^7.6.13. No mismatches detected—no changes required.
packages/language/src/validators/datamodel-validator.ts (1)
110-111: Message wording aligns with "list" terminology elsewhere — LGTMThe error message now consistently uses "List" (matching Line 100: "Optional lists are not supported"). The provider check ('sqlite') remains precise and consistent with runtime dialect providers.
packages/cli/package.json (1)
6-6: Monorepo versions and better-sqlite3 ranges are consistent
- All @zenstackhq/* packages are at 3.0.0-alpha.20.
- Every occurrence of better-sqlite3 in dependencies/devDependencies/peerDependencies is pinned to ^12.2.0.
Please manually verify that the prebuilt binaries for better-sqlite3 support all Node.js versions used in your CI pipelines and downstream consumers to avoid install/runtime issues.
packages/testtools/package.json (1)
3-3: Version/peer bump looks consistent with the coordinated releaseThe version aligns with the repo-wide bump, and peer better-sqlite3 moved to ^12.2.0, consistent with other packages. No further action from my side.
Also applies to: 40-40
packages/runtime/src/client/contract.ts (1)
755-757: Docs clarification for groupBy orderBy/having is accurate and helpfulClarifying that orderBy/having can reference either aggregations or by-fields matches the new aggregation-aware validation and group-by flow. The example using an aggregated filter in having mirrors the implemented behavior.
Also applies to: 762-767
packages/runtime/test/schemas/basic/schema.zmodel (1)
27-27: LGTM: Optional Json meta field added to UserThe addition looks correct and aligns with tests that may want to store arbitrary metadata.
Note: Per repo guidelines, “zenstack generate” must run after schema changes. The generated schema.ts in this PR reflects the update, so this appears handled.
packages/runtime/test/schemas/basic/schema.ts (1)
64-69: LGTM: Generated schema includes the new meta fieldThe generated file correctly mirrors the ZModel change with an optional Json field.
packages/runtime/src/client/crud/operations/group-by.ts (1)
56-58: LGTM: Having now built against subquery aliasUsing the '$sub' alias here keeps having conditions consistent with grouped fields and aggregations sourced from the subquery.
packages/runtime/test/client-api/group-by.test.ts (5)
5-5: LGTM: Imports extended with createPostsPulling in createPosts improves reusability and keeps seeding logic consistent across tests.
36-36: LGTM: Post grouping coverage addedSeeding posts for author '1' and asserting grouping by published with _count exercises the new aggregation paths on an additional model.
Also applies to: 98-109
71-71: LGTM: Use of arrayContaining makes ordering/limits assertions resilientThis avoids over-constraining results when skip/take and ordering interact.
146-157: LGTM: Expanded aggregation coverage with deterministic idsIncluding id in _count/_min/_max and seeding explicit ids ensures deterministic assertions across dialects.
Also applies to: 162-167, 171-179
182-221: LGTM: Having with aggregations is properly exercisedThe test verifies combined aggregate predicates (_avg and _sum) against grouped records.
packages/runtime/src/client/query-utils.ts (2)
288-291: Good reuse: switched to common extractFields utilityConsolidating to the shared extractFields improves consistency and testability.
321-333: Aggregate helper is clear and exhaustiveThe ts-pattern match with exhaustive() guarantees coverage across AGGREGATE_OPERATORS. This is a clean, type-safe mapping to Kysely functions.
packages/runtime/src/client/crud/dialects/base.ts (2)
122-124: Logical combinator guard reads wellThe includes-based check against LOGICAL_COMBINATORS is simple and sufficient for this small set. Good separation via a dedicated type guard.
509-522: Solid: aggregation-aware primitive filtering via matchRouting aggregator keys to aggregate(eb, lhs, op) then reusing buildStandardFilter preserves existing operators/validation without duplication. Nice reuse.
packages/runtime/src/client/crud-types.ts (1)
1065-1068: Nice: GroupByHaving narrows having to aggregation-aware scalar-only whereGood alignment with the validator’s makeHavingSchema and the runtime filter-building logic.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores