refactor: relax prisma type safety to decrease type count #1019
Conversation
📝 WalkthroughWalkthroughThis set of changes primarily comments out and disables all logic related to skipping relationships, models, or properties during Prisma type generation in Rust. Several fields and methods managing such skips are commented out across multiple files, simplifying type generation to always include all relationships and models. Additionally, the order of variants in the Changes
Sequence Diagram(s)sequenceDiagram
participant PrismaTypeGen
participant Model
participant Property
PrismaTypeGen->>Model: Get all relationships/properties
loop For each property
PrismaTypeGen->>Property: Generate type (no skipping logic)
end
PrismaTypeGen-->>Model: Complete type generation (all relationships included)
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (6)
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
src/typegraph/core/src/runtimes/prisma/type_generation/where_.rs (1)
33-41: Remove commented code instead of keeping it.The entire
nestedmethod is commented out rather than removed. Consider removing dead code to improve maintainability.- // fn nested(&self, name: &str, model_id: TypeId) -> Self { - // // let mut skip_models = self.skip_models.clone(); - // // skip_models.insert(self.model_id.name().unwrap().unwrap(), name.to_string()); - // Self { - // model_id, - // // skip_models, - // aggregates: self.aggregates, - // } - // }src/typegraph/core/src/runtimes/prisma/type_generation/order_by.rs (2)
6-6: Clean up commented import.The commented import should be removed if it's no longer needed.
-// use crate::runtimes::prisma::errors;
29-32: Remove commented public method.The commented
skipmethod should be removed rather than kept as commented code.- // pub fn skip(mut self, rel_names: Vec<String>) -> Self { - // self.skip_rel = rel_names; - // self - // }src/typegraph/core/src/runtimes/prisma/type_generation/input_type.rs (1)
54-63: Remove extensive commented relationship handling code.Large blocks of commented code should be removed for better maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (29)
deno.lockis excluded by!**/*.locksrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__query_where_expr__tests__Post__QueryWhereExpr.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__query_where_expr__tests__User__QueryWhereExpr.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__test__create_many Post inp.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__test__create_many Post out.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__test__create_many User inp.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__test__create_many User out.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__test__create_one Post inp.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__test__create_one User inp.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__test__find_first Post out.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__test__find_first User out.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__test__find_many Post inp.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__test__find_many Post out.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__test__find_many User inp.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__test__find_many User out.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__test__find_unique Post inp.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__test__find_unique Post out.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__test__find_unique User inp.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__test__find_unique User out.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__test__group_by Post inp.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__test__group_by Post out.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__test__group_by User inp.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__test__group_by User out.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__test__update_many Post inp.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__test__update_many User inp.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__test__update_one Post inp.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__test__update_one User inp.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__where___test__where Post.snapis excluded by!**/*.snapsrc/typegraph/core/src/runtimes/prisma/type_generation/snapshots/typegraph_core__runtimes__prisma__type_generation__where___test__where User.snapis excluded by!**/*.snap
📒 Files selected for processing (12)
src/meta-cli/src/typegraph/rpc/mod.rs(1 hunks)src/typegraph/core/src/runtimes/prisma/errors.rs(1 hunks)src/typegraph/core/src/runtimes/prisma/type_generation/input_type.rs(5 hunks)src/typegraph/core/src/runtimes/prisma/type_generation/order_by.rs(4 hunks)src/typegraph/core/src/runtimes/prisma/type_generation/out_type.rs(4 hunks)src/typegraph/core/src/runtimes/prisma/type_generation/where_.rs(3 hunks)src/typegraph/core/src/runtimes/prisma/type_generation/with_nested_count.rs(4 hunks)tests/metagen/typegraphs/identities/rs/fdk.rs(1 hunks)tests/metagen/typegraphs/identities/ts/fdk.ts(4 hunks)tests/metagen/typegraphs/sample/ts/client.ts(2 hunks)tests/runtimes/prisma/type_duplication_test.ts(1 hunks)tests/runtimes/wasm_wire/rust/fdk.rs(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
tests/runtimes/prisma/type_duplication_test.ts (1)
tests/runtimes/prisma/type_duplication.ts (1)
tg(7-80)
tests/metagen/typegraphs/identities/ts/fdk.ts (3)
tests/metagen/typegraphs/identities/py/fdk.py (1)
SelectionFlags(433-434)src/metagen/src/client_py/static/client.py (1)
SelectionFlags(421-422)tests/metagen/typegraphs/sample/py/client.py (1)
SelectionFlags(420-421)
src/typegraph/core/src/runtimes/prisma/type_generation/input_type.rs (2)
src/typegraph/core/src/runtimes/prisma/context.rs (1)
model_id(36-38)src/typegraph/core/src/t.rs (2)
prop(404-407)optionalx(261-263)
tests/metagen/typegraphs/sample/ts/client.ts (1)
tests/metagen/typegraphs/sample/py/client.py (1)
SelectionFlags(420-421)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: test-full
- GitHub Check: lint-compat (macos-13, x86_64-apple-darwin, false)
- GitHub Check: lint-compat (macos-14, aarch64-apple-darwin, false)
- GitHub Check: pre-commit
🔇 Additional comments (17)
tests/runtimes/prisma/type_duplication_test.ts (1)
19-19: Improved debugging output for type count monitoring.This change replaces logging of raw serialized data with a concise summary containing the serialized size and type count. This is much more useful for debugging type count issues and monitoring the impact of the PR's changes to reduce type count.
tests/metagen/typegraphs/identities/ts/fdk.ts (1)
1320-1320: LGTM! Consistent addition of selection flags support.The optional
_?: SelectionFlagsproperty is being consistently added to all selection types to support the "select all" functionality. This aligns well with the existing runtime logic in_selectionToNodeSetthat already handles theselectAllflag (line 10:const selectAll = selection._ == "selectAll").Also applies to: 1335-1335, 1346-1346, 1350-1350, 1364-1364, 1369-1369, 1374-1374, 1379-1379, 1384-1384
tests/metagen/typegraphs/sample/ts/client.ts (1)
1111-1111: LGTM! Consistent implementation across different typegraph clients.The addition of
_?: SelectionFlagsto all selection types maintains consistency with the identities client and ensures the "select all" functionality is uniformly available. The existing runtime infrastructure properly supports this feature.Also applies to: 1117-1117, 1131-1131, 1137-1137, 1142-1142, 1146-1146, 1150-1150
tests/metagen/typegraphs/identities/rs/fdk.rs (1)
119-119: LGTM: Version update maintains consistency.The version bump from "0.5.1-rc.2" to "0.5.1-rc.3" aligns with similar updates in other test files and maintains version consistency across the codebase.
tests/runtimes/wasm_wire/rust/fdk.rs (2)
119-119: LGTM: Consistent version update.The version bump to "0.5.1-rc.3" maintains consistency with other test files in the codebase.
393-418: LGTM: Improved type organization.The reorganization of
EntityandProfilestructs and their related type aliases improves code readability by grouping related types together. The types are now positioned more logically afterRecordCreationInputrather than at the end of the module.src/meta-cli/src/typegraph/rpc/mod.rs (1)
23-26:❓ Verification inconclusive
Verify that enum reordering doesn't affect serialization.
Moving
Runtimesto the first position makes logical sense given the PR's focus on runtime changes. However, ensure that this reordering doesn't break existing serialization/deserialization behavior.
🏁 Script executed:
#!/bin/bash # Description: Check if RpcCall enum has untagged serde attribute and verify usage patterns # Expected: Confirm that reordering is safe for existing serialization # Look for RpcCall enum usage and serialization patterns rg -A 10 -B 5 "RpcCall" --type rust | head -50 # Check for any tests that might depend on enum order fd "test" --type f --extension rs --exec rg -l "RpcCall" {} \;Length of output: 3491
Validate untagged
RpcCallvariant orderThe
RpcCallenum insrc/meta-cli/src/typegraph/rpc/mod.rsis annotated with#[serde(untagged)], which means Serde will attempt to deserialize into each variant in declaration order. Reordering these variants can change how incoming payloads are matched and may break existing (de)serialization.• Ensure that moving
Runtimesto the first position doesn’t inadvertently shadow the other variants.
• Add or update unit tests that round-trip serialize/deserialize a representative payload for each variant.
• If any payload now deserializes to the wrong variant, either revert the order or switch to an explicitly tagged representation (#[serde(tag = "...", content = "...")]).src/typegraph/core/src/runtimes/prisma/type_generation/where_.rs (2)
124-147: Simplification of naming logic is consistent with changes.The removal of suffix generation based on
skip_modelsaligns with the overall simplification approach.
20-20:✅ Verification successful
Critical: Removal of skip_models may cause infinite recursion.
The
skip_modelsfield that prevented infinite recursion in self-referencing models has been commented out. This could lead to stack overflow errors during type generation for models with circular relationships.Please verify that this change doesn't introduce infinite recursion by running the following script to check for self-referencing models:
Also applies to: 28-28
🏁 Script executed:
#!/bin/bash # Description: Search for self-referencing model definitions that could cause infinite recursion # Expected: Find models that reference themselves directly or through cycles echo "Searching for potential self-referencing models..." rg -A 10 -B 5 'model.*\{' --type rust | rg -A 15 'Property::Model.*model_type'Length of output: 1364
🏁 Script executed:
#!/bin/bash set -e echo "Displaying skip_models occurrences and nested/generate methods in where_.rs" # List skip_models mentions rg -n skip_models src/typegraph/core/src/runtimes/prisma/type_generation/where_.rs || echo "No skip_models found" # Show nested method context rg -n "fn nested" -C5 src/typegraph/core/src/runtimes/prisma/type_generation/where_.rs || echo "No nested method found" # Show generate method implementation rg -n "fn generate" -A20 src/typegraph/core/src/runtimes/prisma/type_generation/where_.rs || echo "Generate method not found" # Display the file header to inspect overall context sed -n '1,200p' src/typegraph/core/src/runtimes/prisma/type_generation/where_.rsLength of output: 8823
🏁 Script executed:
#!/bin/bash set -e echo "=== PrismaContext struct definition ===" rg -n "struct PrismaContext" -A20 -B5 src/typegraph/core/src/runtimes/prisma/context.rs || echo "PrismaContext struct not found" echo echo "=== impl PrismaContext and generate methods ===" rg -n "impl PrismaContext" -A200 src/typegraph/core/src/runtimes/prisma/context.rs || echo "impl PrismaContext not found" echo rg -n "fn generate" -A50 src/typegraph/core/src/runtimes/prisma/context.rs || echo "generate method not found"Length of output: 9413
🏁 Script executed:
#!/bin/bash set -e echo "Inspecting typegen_cache usage and generate method in PrismaContext" rg -n "typegen_cache" -C10 src/typegraph/core/src/runtimes/prisma/context.rs || echo "typegen_cache not found" rg -n "fn generate<" -C5 src/typegraph/core/src/runtimes/prisma/context.rs || echo "Generic generate method not found" rg -n "get_or_init" -C5 src/typegraph/core/src/runtimes/prisma/context.rs || echo "No get_or_init usage found"Length of output: 1287
🏁 Script executed:
#!/bin/bash set -e echo "Searching for TypeGen trait definition" rg -n "trait TypeGen" -A5 src/typegraph/core/src/runtimes/prisma || echo "TypeGen trait not found"Length of output: 729
🏁 Script executed:
#!/bin/bash set -e echo "=== type_generation/mod.rs (first 200 lines) ===" sed -n '1,200p' src/typegraph/core/src/runtimes/prisma/type_generation/mod.rsLength of output: 6438
No infinite recursion risk: PrismaContext’s typegen_cache prevents it
The
skip_modelslogic has been removed, but recursion is still guarded by the cache inPrismaContext::generate. It pre-inserts a reference placeholder for eachtype_namebefore recursing, so nested calls for the same or cyclic models immediately hit the cache and return aref_, rather than recursing further.No changes required here.
src/typegraph/core/src/runtimes/prisma/type_generation/order_by.rs (2)
16-16: Consistent removal of skip mechanism across type generators.The
skip_relfield removal is consistent with similar changes in other type generation modules.Also applies to: 24-24
56-62:❓ Verification inconclusive
Risk of infinite recursion in OrderBy generation.
Similar to the Where type, removing the skip logic could cause infinite recursion when generating OrderBy types for self-referencing models.
The same recursion risks apply here as in the Where type. Please ensure this change has been tested with circular model relationships.
Also applies to: 70-73
🏁 Script executed:
#!/bin/bash # Search for skip_rel references in order_by.rs rg 'skip_rel' -n src/typegraph/core/src/runtimes/prisma/type_generation/order_by.rs || echo "no skip_rel found in order_by.rs" # Show context around the commented-out skip_rel logic removal sed -n '50,80p' src/typegraph/core/src/runtimes/prisma/type_generation/order_by.rs echo "" echo "=== Comparing Where generation for skip_rel ===" rg 'skip_rel' -n src/typegraph/core/src/runtimes/prisma/type_generation/where.rs || echo "no skip_rel found in where.rs" # Show context around skip_rel usage in where.rs sed -n '50,80p' src/typegraph/core/src/runtimes/prisma/type_generation/where.rsLength of output: 2666
Verify infinite recursion risk in OrderBy generator
The
skip_relchecks were removed from your OrderBy generator in
src/typegraph/core/src/runtimes/prisma/type_generation/order_by.rs
– specifically around lines 56–62 and 70–73. Without these guards, self-referencing models can trigger infinite recursion when building OrderBy types.• Please add tests covering circular relationships
• Or re-introduce theskip_rellogic (as used in other generators) to break cyclessrc/typegraph/core/src/runtimes/prisma/type_generation/with_nested_count.rs (2)
16-16: Consistent with skip removal pattern.The removal of the
skipfield follows the same pattern as other type generation modules.Also applies to: 23-23
38-48:❓ Verification inconclusive
Comprehensive removal of relationship skip logic.
The commented code shows extensive logic for handling relationship skipping and self-referencing models. This was likely important for preventing infinite recursion.
Please verify that models with self-references (where
rel.left.model_type == rel.right.model_type) don't cause infinite loops in the simplified implementation.
🏁 Script executed:
#!/usr/bin/env bash set -euo pipefail # Locate with_nested_count.rs file=$(fd --full-path "with_nested_count.rs" | head -n1) echo "Inspecting file: $file" # Print the first 200 lines to understand the current logic sed -n '1,200p' "$file"Length of output: 4346
🏁 Script executed:
#!/usr/bin/env bash set -euo pipefail # Locate PrismaContext file file=$(fd --full-path "context.rs" | grep "src/typegraph/core/src/runtimes/prisma/context.rs" | head -n1) echo "Inspecting context file: $file" # Show implementation of PrismaContext including generate method sed -n '1,200p' "$file" # Search for generate method definition rg -n "fn generate" -C5 "$file"Length of output: 7047
🏁 Script executed:
#!/usr/bin/env bash set -euo pipefail # Search for generate method in PrismaContext rg -n "fn generate" -C2 src/typegraph/core/src/runtimes/prisma/context.rs || true # Search for impl TypegraphContext in Prisma runtime rg -n "impl.*TypegraphContext" -C3 src/typegraph/core/src/runtimes/prismaLength of output: 178
🏁 Script executed:
#!/usr/bin/env bash set -euo pipefail # Search for TypegraphContext trait definition and its implementations rg -n "trait TypegraphContext" -C3 . rg -n "impl .*TypegraphContext" -C3 .Length of output: 58
Verify prevention of infinite recursion for self-referential models
I wasn’t able to locate how
context.generateshort-circuits on previously built types (e.g. via thetypegen_cache) to confirm that a model referencing itself won’t endlessly recurse. Please ensure that either:
- The
TypegraphContext::generateimplementation checks the type’s name cache (so repeated calls forModel_with_nested_countreturn the cachedTypeIdrather than rebuild), or- Add back the self-reference skip logic (i.e. when
rel.left.model_type == rel.right.model_type) to prevent infinite nesting.src/typegraph/core/src/runtimes/prisma/type_generation/input_type.rs (3)
23-23: Consistent removal of skip_rel mechanism.The removal of
skip_relfield and initialization is consistent with other type generation modules.Also applies to: 31-31, 39-39
218-230: Simplified naming removes complexity tracking.The naming logic has been simplified to remove skip-based suffixes, which is consistent with the overall approach.
145-150:❓ Verification inconclusive
Breaking change: All relationships now optional in create operations.
This is a significant behavioral change where relationships that were previously required in create operations are now always optional. This aligns with the PR objective but represents a breaking change.
Please verify that making all relationships optional doesn't break existing functionality. Run the following script to check for tests that might be affected:
🏁 Script executed:
#!/bin/bash # Description: Find tests that might be affected by making relationships optional # Expected: Find test cases that rely on required relationships echo "Searching for tests that might assume required relationships..." rg -A 5 -B 5 'create.*input|InputType.*create' --type rust tests/ rg -A 10 'required.*relation|relation.*required' --type rust tests/Length of output: 274
Verify impact of making relationships optional in create operations
The change unconditionally wraps all relationship fields in
t::optionalx(inner), removing the prior special-case for(Operation::Create, Cardinality::One). This is a breaking behavioral change: fields that were once required on create are now optional.I couldn’t find any tests explicitly assuming required relationships (no matches in
tests/), but that absence isn’t proof there are none. Please manually verify:• Review your integration and unit tests (both in
tests/andsrc/**/mod.rsor*_test.rs) for any create-mutation scenarios that depended on non-optional relations.
• Confirm generated GraphQL input types still match your schema expectations for create operations.
• Run end-to-end or schema-introspection tests to catch any consumption errors.File to inspect:
src/typegraph/core/src/runtimes/prisma/type_generation/input_type.rs(around lines 145–150)src/typegraph/core/src/runtimes/prisma/type_generation/out_type.rs (1)
79-95:❓ Verification inconclusive
Potential type name conflicts without unique suffixes.
Removing the suffix generation means all output types for a model will have the same name regardless of which relationships are included. If the system previously generated multiple type variations with different relationships, this could lead to naming conflicts.
Run the following script to check if the codebase relies on generating multiple type variations:
🏁 Script executed:
#!/bin/bash # Description: Check for patterns that might indicate reliance on multiple type variations # Search for places where OutType is instantiated with different skip_rel sets ast-grep --pattern 'OutType { $$$, skip_rel: $_ }' # Search for references to output type names with suffixes rg -A 2 '_output_excluding_'Length of output: 81
Confirm skip_rel usage to avoid naming collisions
Our search didn’t find any instantiations of
OutTypewith non-emptyskip_relor any references to the old_excluding_suffix. However, absence of evidence isn’t evidence of absence. Please manually verify whether anywhere in the Prisma type-generation pipeline multipleOutTypevalues are created with differingskip_relsets. If they are, you’ll need to restore unique suffix logic to prevent GraphQL type name conflicts.
src/typegraph/core/src/runtimes/prisma/type_generation/with_nested_count.rs
Show resolved
Hide resolved
Migration notes
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests