Skip to content

Conversation

@olup
Copy link
Contributor

@olup olup commented Dec 2, 2025

Summary

  • Fixes array schema generation for TypeDef[] @json fields directly on model fields
  • Uses field.type.array from ZModel AST instead of def.isList from DMMF
  • Adds test case for array of TypeDef with enum directly on model field

Problem

When a model field uses TypeDef[] @json directly:

type TranslatedField {
    language Language
    content String
}

model Article {
    title TranslatedField[] @json
}

The OpenAPI generator was producing:

"title": { "$ref": "#/components/schemas/TranslatedField" }

Instead of the correct:

"title": { "type": "array", "items": { "$ref": "#/components/schemas/TranslatedField" } }

Root Cause

The code was using def.isList from Prisma's DMMF, but Prisma treats all @json fields as plain Json type and doesn't preserve the [] array notation from ZModel. The array information is only available in the ZModel AST via field.type.array.

Test plan

  • Added new test case: array of TypeDef with enum directly on model field
  • All existing OpenAPI tests pass (24/24)

🤖 Generated with Claude Code

ymc9 added 30 commits November 22, 2024 11:59
ymc9 and others added 17 commits July 7, 2025 11:56
When a model field uses `TypeDef[] @json` directly (e.g., `title TranslatedField[] @json`),
the OpenAPI generator was not generating the correct array schema. Instead of:

```json
"title": { "type": "array", "items": { "$ref": "#/components/schemas/TranslatedField" } }
```

It was generating:

```json
"title": { "$ref": "#/components/schemas/TranslatedField" }
```

The bug was that `def.isList` comes from Prisma's DMMF, but Prisma treats all `@json`
fields as plain `Json` type and doesn't know about the `[]` array notation in ZModel.
The array information is only available in the ZModel AST via `field.type.array`.

This fix uses `field.type.array` from the ZModel AST instead of `def.isList` from DMMF
when generating OpenAPI schemas for TypeDef-referenced Json fields.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

coderabbitai bot commented Dec 2, 2025

📝 Walkthrough

Walkthrough

Modified array detection for Json fields referencing TypeDefs in OpenAPI RPC generation. Changed from using def.isList from DMMF to field.type.array from the ZModel AST. Added test case for array of TypeDef with enum validation (duplicated in test file).

Changes

Cohort / File(s) Summary
Core logic update
packages/plugins/openapi/src/rpc-generator.ts
Changed array-detection mechanism for TypeDef-backed Json fields to use field.type.array from AST instead of def.isList from DMMF. Wrapping order for nullable, reference, and array handling remains unchanged.
Test coverage
packages/plugins/openapi/tests/openapi-rpc.test.ts
Added test case "array of TypeDef with enum directly on model field" validating OpenAPI spec generation, TypeDef references, enum generation, and correct $ref relationships for both 3.0.0 and 3.1.0 spec versions. Test block appears duplicated in file.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Areas requiring attention:
    • Verify the semantic difference between field.type.array (AST) and def.isList (DMMF) and confirm the change correctly handles edge cases for TypeDef-backed Json fields
    • Check that nullable and reference wrapping logic still functions correctly with the array-detection source change
    • Note the duplicated test block in the test file—clarify if intentional or should be removed

Possibly related PRs

  • zenstackhq/zenstack#2222: Also modifies rpc-generator.ts to change array detection for Json fields referencing TypeDefs, using AST TypeDef information instead of DMMF.

Suggested reviewers

  • ymc9

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and specifically describes the main fix: using ZModel AST array flag instead of DMMF for TypeDef[] @JSON fields.
Description check ✅ Passed The description is directly related to the changeset, clearly explaining the problem, root cause, solution, and test coverage for the TypeDef[] @JSON array schema generation fix.
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

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b53b1cb and b8ffbbd.

📒 Files selected for processing (2)
  • packages/plugins/openapi/src/rpc-generator.ts (1 hunks)
  • packages/plugins/openapi/tests/openapi-rpc.test.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/plugins/openapi/tests/openapi-rpc.test.ts (3)
packages/testtools/src/schema.ts (2)
  • loadZModelAndDmmf (400-432)
  • normalizePath (72-74)
packages/plugins/openapi/src/rpc-generator.ts (1)
  • generate (55-115)
packages/plugins/openapi/src/rest-generator.ts (1)
  • generate (59-107)
⏰ 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). (4)
  • GitHub Check: build-test (20.x)
  • GitHub Check: dependency-review
  • GitHub Check: build-test (20.x)
  • GitHub Check: build-test (20.x)
🔇 Additional comments (2)
packages/plugins/openapi/src/rpc-generator.ts (1)

788-792: LGTM! Correct fix for array detection.

The fix correctly uses field.type.array from the ZModel AST instead of def.isList from DMMF for Json fields referencing TypeDefs. This is necessary because Prisma treats TypeDef[] @json fields as plain Json type, losing the array semantics. The change is properly scoped and preserves the correct wrapping order (nullable → reference → array).

packages/plugins/openapi/tests/openapi-rpc.test.ts (1)

521-578: Verify test duplication mentioned in the summary.

The test case looks comprehensive and properly validates the fix for array detection of TypeDef fields. However, the AI summary states: "The new test block appears twice in the file (duplicated), each performing identical setup and assertions for the same scenario."

I don't see duplication in the provided code. Please verify if this test appears elsewhere in the file and remove any duplicate if it exists.


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.

@ymc9 ymc9 changed the base branch from main to dev December 4, 2025 00:31
Copy link
Member

@ymc9 ymc9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you for making the fix @olup

@ymc9 ymc9 merged commit 50bf7b2 into zenstackhq:dev Dec 4, 2025
8 checks passed
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.

2 participants