Conversation
📝 WalkthroughWalkthroughThis PR refactors Strawchemy's pagination, ordering, and field-filtering APIs to use explicit include/exclude patterns instead of boolean flags. Core changes include replacing Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/strawchemy/schema/factories/base.py (2)
73-77: Remove unusedChildOptionsclass.The
ChildOptionsclass appears to be dead code left over from the refactoring. It's no longer referenced anywhere in the codebase after the migration to explicitpaginateandorderparameters.🔎 Proposed removal
-@dataclasses.dataclass(eq=True, frozen=True) -class ChildOptions: - pagination: DefaultOffsetPagination | bool = False - order_by: bool = False - - class GraphQLDTOFactory(DTOFactory[DeclarativeBase, QueryableAttribute[Any], GraphQLDTOT]):
93-124: Critical:frozensetconversion incorrectly handles the "all" literal.Lines 114-115 convert
orderandpaginatetofrozenset, but when these parameters contain the string"all",frozenset("all")createsfrozenset({'a', 'l'})instead of a proper representation of "all fields".This will cause incorrect behavior when
paginate="all"ororder="all"is used, as the registry will receive a malformed frozenset.🔎 Proposed fix
def _type_info( self, dto: type[StrawchemyDTOT], dto_config: DTOConfig, current_node: Node[Relation[Any, GraphQLDTOT], None] | None, override: bool = False, user_defined: bool = False, order: IncludeFields | None = None, paginate: IncludeFields | None = None, default_pagination: None | DefaultOffsetPagination = None, ) -> RegistryTypeInfo: graphql_type = self.graphql_type(dto_config) model: type[DeclarativeBase] | None = dto.__dto_model__ if issubclass(dto, MappedStrawberryGraphQLDTO) else None # type: ignore[reportGeneralTypeIssues] default_name = self.root_dto_name(model, dto_config, current_node) if model else dto.__name__ + + def to_frozenset(value: IncludeFields | None) -> frozenset[str]: + if value is None: + return frozenset() + if value == "all": + return frozenset(["all"]) + return frozenset(value) + type_info = RegistryTypeInfo( name=dto.__name__, default_name=default_name, graphql_type=graphql_type, override=override, user_defined=user_defined, pagination=default_pagination, - order=frozenset() if order is None else frozenset(order), - paginate=frozenset() if paginate is None else frozenset(paginate), + order=to_frozenset(order), + paginate=to_frozenset(paginate), scope=dto_config.scope, model=model, exclude_from_scope=dto_config.exclude_from_scope, )
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (8)
tests/unit/__snapshots__/test_example_app/test_graphql_schema.gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[all_fields_order_by].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[order_empty_list].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[order_specific_fields].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[paginate_all_with_default].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[paginate_and_order_combined].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[paginate_empty_list].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[paginate_specific_fields].gqlis excluded by!**/__snapshots__/**
📒 Files selected for processing (24)
README.mdsrc/strawchemy/dto/types.pysrc/strawchemy/schema/factories/base.pysrc/strawchemy/schema/factories/inputs.pysrc/strawchemy/schema/factories/types.pysrc/strawchemy/schema/field.pysrc/strawchemy/utils/registry.pytests/integration/types/mysql.pytests/integration/types/postgres.pytests/integration/types/sqlite.pytests/unit/dto/test_dto.pytests/unit/mapping/test_schemas.pytests/unit/models.pytests/unit/schemas/order/auto_order_by.pytests/unit/schemas/order/order_empty_list.pytests/unit/schemas/order/order_specific_fields.pytests/unit/schemas/override/override_argument.pytests/unit/schemas/pagination/children_pagination.pytests/unit/schemas/pagination/children_pagination_defaults.pytests/unit/schemas/pagination/paginate_all_with_default.pytests/unit/schemas/pagination/paginate_and_order_combined.pytests/unit/schemas/pagination/paginate_empty_list.pytests/unit/schemas/pagination/paginate_specific_fields.pytests/unit/schemas/pagination/pagination_default_limit.py
🧰 Additional context used
🧬 Code graph analysis (18)
tests/integration/types/sqlite.py (1)
src/strawchemy/transpiler/_query.py (1)
order(100-102)
tests/unit/schemas/override/override_argument.py (2)
tests/fixtures.py (1)
strawchemy(34-35)src/strawchemy/transpiler/_query.py (1)
order(100-102)
tests/unit/models.py (3)
tests/unit/dc_models.py (1)
UUIDBase(14-15)src/strawchemy/dto/base.py (1)
name(318-319)tests/integration/models.py (2)
Fruit(127-146)Color(149-155)
tests/integration/types/postgres.py (4)
tests/fixtures.py (1)
strawchemy(34-35)tests/unit/models.py (2)
Fruit(56-64)Color(81-85)tests/integration/models.py (2)
Fruit(127-146)Color(149-155)src/strawchemy/transpiler/_query.py (1)
order(100-102)
tests/unit/dto/test_dto.py (1)
src/strawchemy/dto/types.py (4)
DTOConfig(115-276)from_include(189-190)Purpose(67-87)is_field_included(273-276)
tests/unit/schemas/order/order_empty_list.py (1)
tests/unit/schemas/order/order_specific_fields.py (2)
ContainerType(12-13)Query(17-18)
tests/unit/schemas/pagination/pagination_default_limit.py (3)
tests/fixtures.py (1)
strawchemy(34-35)tests/unit/models.py (1)
Fruit(56-64)tests/integration/models.py (1)
Fruit(127-146)
tests/integration/types/mysql.py (2)
tests/fixtures.py (1)
strawchemy(34-35)src/strawchemy/transpiler/_query.py (1)
order(100-102)
src/strawchemy/schema/factories/types.py (2)
src/strawchemy/dto/types.py (4)
DTOConfig(115-276)Purpose(67-87)from_include(189-190)is_field_included(273-276)src/strawchemy/schema/pagination.py (1)
DefaultOffsetPagination(9-11)
tests/unit/schemas/pagination/children_pagination.py (3)
tests/fixtures.py (1)
strawchemy(34-35)tests/unit/models.py (1)
Fruit(56-64)tests/integration/models.py (1)
Fruit(127-146)
tests/unit/schemas/pagination/paginate_all_with_default.py (1)
src/strawchemy/schema/pagination.py (1)
DefaultOffsetPagination(9-11)
src/strawchemy/utils/registry.py (2)
src/strawchemy/schema/pagination.py (1)
DefaultOffsetPagination(9-11)src/strawchemy/schema/factories/base.py (1)
graphql_type(323-324)
tests/unit/schemas/pagination/children_pagination_defaults.py (2)
tests/fixtures.py (1)
strawchemy(34-35)src/strawchemy/schema/pagination.py (1)
DefaultOffsetPagination(9-11)
tests/unit/schemas/pagination/paginate_empty_list.py (4)
tests/fixtures.py (1)
strawchemy(34-35)tests/unit/models.py (1)
Container(200-212)tests/unit/schemas/order/order_empty_list.py (2)
ContainerType(12-13)Query(17-18)tests/unit/schemas/pagination/paginate_specific_fields.py (2)
ContainerType(12-13)Query(17-18)
src/strawchemy/schema/factories/inputs.py (2)
src/strawchemy/schema/factories/types.py (1)
_aggregation_field(89-104)src/strawchemy/dto/types.py (1)
copy_with(192-232)
tests/unit/schemas/pagination/paginate_specific_fields.py (3)
tests/fixtures.py (1)
strawchemy(34-35)tests/unit/models.py (1)
Container(200-212)tests/unit/schemas/pagination/paginate_all_with_default.py (1)
Query(18-19)
src/strawchemy/dto/types.py (1)
src/strawchemy/dto/base.py (1)
name(318-319)
tests/unit/schemas/order/auto_order_by.py (3)
tests/unit/models.py (1)
Group(151-159)tests/integration/models.py (1)
Group(158-162)src/strawchemy/transpiler/_query.py (1)
order(100-102)
⏰ 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). (19)
- GitHub Check: 🔬 integration-postgres on 3.12
- GitHub Check: 🔬 integration-sqlite on 3.11
- GitHub Check: 🔬 integration-mysql on 3.10
- GitHub Check: 🔬 integration-postgres on 3.14
- GitHub Check: 🔬 integration-mysql on 3.13
- GitHub Check: 🔬 integration-sqlite on 3.10
- GitHub Check: 🔬 integration-sqlite on 3.14
- GitHub Check: 🔬 integration-postgres on 3.10
- GitHub Check: 🔬 integration-postgres on 3.13
- GitHub Check: 🔬 integration-postgres on 3.11
- GitHub Check: 🔬 integration-mysql on 3.12
- GitHub Check: 🔬 integration-sqlite on 3.13
- GitHub Check: 🔬 unit-no-extras on 3.12
- GitHub Check: 🔬 unit on 3.12
- GitHub Check: 🔬 unit-no-extras on 3.10
- GitHub Check: 🔬 unit on 3.10
- GitHub Check: 🔬 unit on 3.13
- GitHub Check: 🔬 unit on 3.11
- GitHub Check: Optimize new code in this PR
🔇 Additional comments (31)
tests/unit/mapping/test_schemas.py (2)
201-202: LGTM! Order test coverage expanded.The two new order test cases (
order_specific_fields,order_empty_list) follow the established pattern and are well-placed with other order tests. Theorder_empty_listcase is particularly valuable for edge case testing.
189-192: LGTM! Comprehensive pagination test coverage added.The four new pagination test cases (
paginate_specific_fields,paginate_empty_list,paginate_and_order_combined,paginate_all_with_default) follow the existing pattern and expand coverage for the refactored pagination API. All corresponding test schema modules exist at the specified paths, and the naming is clear and descriptive.tests/unit/schemas/order/order_empty_list.py (4)
1-6: LGTM: Imports are well-structured.The imports are appropriate for a Strawchemy test schema module, with future annotations enabled for better type hint support.
8-8: LGTM: Strawchemy instance initialization.The PostgreSQL-specific Strawchemy instance is correctly initialized for use in the schema definitions.
16-18: LGTM: Query type definition is correct.The Query type correctly defines a container field returning a list of ContainerType using the Strawchemy field helper.
11-13: This is a test fixture file tested by the centralized snapshot test intests/unit/mapping/test_schemas.py.The schema definition with
order=[]is validated through the parametrizedtest_query_schemasfunction, which generates and compares the GraphQL schema against snapshots. This pattern is consistent across the other schema files in the same directory.Likely an incorrect or invalid review comment.
src/strawchemy/schema/field.py (1)
115-115: LGTM: Minor formatting improvement.The blank line improves readability by visually separating the pagination initialization logic from the subsequent field assignments.
tests/unit/schemas/pagination/paginate_all_with_default.py (1)
1-19: LGTM: Test correctly demonstrates custom pagination defaults.This test file properly demonstrates the new
paginate="all"API with a customDefaultOffsetPaginationconfiguration. The implementation aligns with the PR's objective to provide explicit pagination control.src/strawchemy/utils/registry.py (1)
104-106: LGTM: Enhanced API for granular pagination and ordering control.The refactored
RegistryTypeInfofields replace boolean flags with more flexible configurations:
paginationnow accepts an optionalDefaultOffsetPaginationinstance (rather thanbool | DefaultOffsetPagination)orderandpaginateaccept either"all"or afrozensetof specific field namesThis enables per-field control over pagination and ordering, which is a significant improvement over the previous boolean approach. The use of
frozensetwithdefault_factorycorrectly prevents shared mutable defaults.tests/unit/schemas/order/order_specific_fields.py (1)
1-18: LGTM: Test correctly demonstrates selective field ordering.This test properly showcases the new
orderparameter with an explicit field list["fruits", "vegetables"], enabling ordering for only those specific fields while excluding others (e.g., "colors"). This aligns with the PR's objective to provide granular control over ordering behavior.tests/unit/dto/test_dto.py (1)
269-328: LGTM: Comprehensive test coverage for new DTOConfig methods.The test suite thoroughly covers the new
from_include()andis_field_included()methods with multiple scenarios:
- Different input types (
None,"all",list,set)- Custom
Purposevalues- Edge cases (empty include, non-existent fields)
This comprehensive coverage ensures the new inclusion logic behaves correctly across all use cases.
tests/unit/schemas/pagination/pagination_default_limit.py (1)
11-11: LGTM: Updated to use new explicit pagination API.The change from
child_pagination=Truetopaginate="all"correctly migrates to the new explicit pagination API, making the intent clearer while maintaining the same functionality.src/strawchemy/schema/factories/inputs.py (1)
146-148: LGTM: Correctly enables partial field specification for aggregate filters.Adding
partial=Trueto thedto_config.copy_with()call ensures that aggregate filter fields are treated as optional, which aligns with filter semantics where users should be able to specify only the fields they want to filter on. This change complements thepartial_default=UNSETparameter for proper handling of unspecified filter criteria.tests/unit/models.py (1)
200-213: No action needed—these relationships are intentionally arbitrary and designed for schema generation testing.The
Containermodel's relationships withprimaryjoinconditions are intentionally configured for testing Strawchemy's pagination/ordering schema generation with multiple list relationships. These are snapshot tests (intest_query_schemas) that verify the generated GraphQL schema structure, not integration tests that query actual data. Theviewonly=Truesetting is appropriate, and test fixtures that populate matching data are not required because the relationships are never used to execute queries.tests/unit/schemas/override/override_argument.py (1)
11-11: LGTM! API update is clear and consistent.The change from
child_pagination=True, child_order_by=Truetopaginate="all", order="all"makes the pagination and ordering controls more explicit and flexible.README.md (1)
600-600: LGTM! Documentation updated correctly.The README example now reflects the new
paginate="all"parameter, helping users understand the updated API for enabling pagination on nested relationships.tests/unit/schemas/order/auto_order_by.py (1)
11-11: LGTM! Ordering API update is consistent.The change from
child_order_by=Truetoorder="all"aligns with the new API design, making ordering controls more explicit.tests/integration/types/sqlite.py (1)
153-153: LGTM! Integration test updates are consistent.All three type declarations correctly adopt the new pagination and ordering parameters:
paginate="all"replaceschild_pagination=Trueorder="all"replaceschild_order_by=TrueThe changes are consistent with the broader API refactoring.
Also applies to: 184-184, 196-196
tests/unit/schemas/pagination/children_pagination.py (1)
11-11: LGTM! Pagination test updated correctly.The test now uses
paginate="all"instead ofchild_pagination=True, aligning with the new API.tests/unit/schemas/pagination/children_pagination_defaults.py (1)
12-12: LGTM! New pagination defaults API is clearer.The change introduces separate
paginate="all"anddefault_pagination=DefaultOffsetPagination(...)parameters, which is more explicit than the previous approach wherechild_paginationcould be either a boolean or a pagination config.src/strawchemy/dto/types.py (1)
188-190: LGTM! Helper classmethod is straightforward.The
from_includeclassmethod provides a convenient way to construct aDTOConfigwith an include parameter, defaulting to an empty set whenincludeis None.tests/unit/schemas/pagination/paginate_specific_fields.py (1)
11-13: LGTM! New API enables granular pagination control.This test demonstrates a key improvement in the new API:
paginate=["fruits"]allows enabling pagination on specific fields rather than all-or-nothing. This provides much finer-grained control compared to the previouschild_pagination=True.tests/unit/schemas/pagination/paginate_empty_list.py (1)
1-18: LGTM! Test file correctly demonstrates the newpaginateAPI.The test file follows the established pattern and correctly uses
paginate=[]to demonstrate disabling pagination for all fields.tests/integration/types/postgres.py (1)
162-163: LGTM! Integration test types correctly updated to new pagination/ordering API.The changes consistently replace the old
child_pagination/child_order_byboolean flags with the new explicitpaginateandorderparameters. This aligns with the PR's refactoring goals and matches the pattern used in other integration test files.Also applies to: 193-194, 205-206
tests/unit/schemas/pagination/paginate_and_order_combined.py (1)
1-33: LGTM! Excellent test coverage for combined pagination and ordering scenarios.This test file demonstrates various combinations:
- Different fields for pagination vs ordering
- Overlapping field specifications
- Mixing "all" with specific field lists
This provides good coverage for the new API's flexibility.
tests/integration/types/mysql.py (1)
154-155: LGTM! MySQL integration tests updated consistently with PostgreSQL tests.The changes mirror those in
tests/integration/types/postgres.py, ensuring consistent behavior across database backends.Also applies to: 185-186, 197-198
src/strawchemy/schema/factories/types.py (2)
106-160: LGTM! Refactored method correctly implements per-field pagination and ordering controls.The renamed
_add_fields_argumentsmethod (formerly_update_fields) properly:
- Uses
DTOConfig.from_include()to create configuration objects from theorderandpaginateparameters- Checks field inclusion via
is_field_included()before applying pagination or ordering- Adds JSON path filtering support for JSON/dict fields (lines 134-147)
The implementation correctly handles the various cases:
"all", specific field lists, andNone.
193-251: LGTM! Factory method signature correctly updated with new pagination/ordering parameters.The changes properly:
- Add
default_pagination,order, andpaginateparameters to the factory signature- Pass
"all"values through tosuper().factory()while converting other values toNone(lines 229-231)- Pass original parameter values to
_add_fields_arguments(line 236) and_register_type(lines 246-248)This allows the factory to handle both DTO-level (via super) and field-level (via
_add_fields_arguments) pagination/ordering configuration.src/strawchemy/schema/factories/base.py (3)
210-268: LGTM! Type wrapper correctly threads new pagination and ordering parameters.The
_type_wrappermethod properly:
- Accepts the new
paginate,order, anddefault_paginationparameters (lines 221-223)- Passes them through to the factory call (lines 257-259)
The decorator pattern is correctly maintained.
437-483: LGTM! Publictypemethod correctly exposes new pagination/ordering API.The method signature properly adds the new parameters (lines 448-450) and passes them to
_type_wrapper(lines 470-472), making the new API available to users.
592-634: LGTM! Unmapped DTO factorytypemethod updated consistently.The unmapped factory's
typemethod includes the same parameter additions (lines 601-603) and passes them through correctly (lines 622-624), ensuring API consistency across factory types.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #147 +/- ##
==========================================
- Coverage 94.28% 94.18% -0.11%
==========================================
Files 69 69
Lines 5814 5915 +101
Branches 770 793 +23
==========================================
+ Hits 5482 5571 +89
- Misses 190 199 +9
- Partials 142 145 +3 ☔ View full report in Codecov by Sentry. |
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @gazorby. * #147 (comment) The following files were modified: * `src/strawchemy/dto/types.py` * `src/strawchemy/schema/factories/base.py` * `src/strawchemy/schema/factories/inputs.py` * `src/strawchemy/schema/factories/types.py` * `src/strawchemy/schema/field.py` * `src/strawchemy/utils/registry.py`
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/strawchemy/schema/factories/base.py (1)
83-86: Remove unusedChildOptionsclass or document its purpose.The
ChildOptionsdataclass is defined but never instantiated or referenced anywhere in the codebase—only re-exported as part of the public API. If this is a leftover from refactoring (the comment suggests it may relate to removedchild_paginationandchild_order_bylogic), consider either removing it or documenting why it's retained for external API stability.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[pagination_default_offset].gqlis excluded by!**/__snapshots__/**
📒 Files selected for processing (14)
src/strawchemy/config/base.pysrc/strawchemy/dto/base.pysrc/strawchemy/dto/types.pysrc/strawchemy/dto/utils.pysrc/strawchemy/mapper.pysrc/strawchemy/schema/factories/base.pysrc/strawchemy/schema/factories/enum.pysrc/strawchemy/schema/factories/inputs.pysrc/strawchemy/schema/factories/types.pysrc/strawchemy/schema/field.pysrc/strawchemy/utils/registry.pysrc/strawchemy/validation/pydantic.pytests/unit/mapping/test_schemas.pytests/unit/schemas/pagination/pagination_default_offset.py
🧰 Additional context used
🧬 Code graph analysis (9)
src/strawchemy/mapper.py (1)
src/strawchemy/schema/pagination.py (1)
DefaultOffsetPagination(9-11)
src/strawchemy/validation/pydantic.py (1)
src/strawchemy/dto/types.py (2)
DTOConfig(115-337)Purpose(67-87)
src/strawchemy/utils/registry.py (2)
src/strawchemy/schema/pagination.py (1)
DefaultOffsetPagination(9-11)src/strawchemy/mapper.py (3)
field(167-193)field(196-221)field(223-329)
src/strawchemy/dto/types.py (3)
src/strawchemy/dto/utils.py (1)
field(72-83)src/strawchemy/mapper.py (3)
field(167-193)field(196-221)field(223-329)src/strawchemy/dto/base.py (1)
name(318-319)
src/strawchemy/schema/factories/types.py (1)
src/strawchemy/dto/types.py (4)
DTOConfig(115-337)Purpose(67-87)from_include(189-216)is_field_included(299-337)
src/strawchemy/schema/factories/enum.py (1)
src/strawchemy/dto/types.py (2)
DTOConfig(115-337)Purpose(67-87)
tests/unit/schemas/pagination/pagination_default_offset.py (2)
src/strawchemy/mapper.py (4)
Strawchemy(61-693)field(167-193)field(196-221)field(223-329)src/strawchemy/config/base.py (1)
StrawchemyConfig(18-63)
src/strawchemy/schema/factories/inputs.py (2)
src/strawchemy/schema/factories/types.py (1)
_aggregation_field(89-104)src/strawchemy/dto/types.py (1)
copy_with(218-258)
src/strawchemy/config/base.py (1)
src/strawchemy/schema/field.py (1)
order_by(235-240)
🪛 Ruff (0.14.10)
src/strawchemy/dto/types.py
189-189: Boolean-typed positional argument in function definition
(FBT001)
src/strawchemy/schema/factories/base.py
69-69: Boolean-typed positional argument in function definition
(FBT001)
340-340: Dynamically typed expressions (typing.Any) are disallowed in **kwargs
(ANN401)
647-647: Boolean-typed positional argument in function definition
(FBT001)
⏰ 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). (20)
- GitHub Check: 🔬 integration-mysql on 3.13
- GitHub Check: 🔬 integration-postgres on 3.14
- GitHub Check: 🔬 integration-sqlite on 3.12
- GitHub Check: 🔬 integration-mysql on 3.12
- GitHub Check: 🔬 integration-mysql on 3.11
- GitHub Check: 🔬 integration-postgres on 3.13
- GitHub Check: 🔬 unit-no-extras on 3.14
- GitHub Check: 🔬 unit-no-extras on 3.11
- GitHub Check: 🔬 integration-mysql on 3.14
- GitHub Check: 🔬 integration-postgres on 3.12
- GitHub Check: 🔬 integration-sqlite on 3.14
- GitHub Check: 🔬 unit on 3.14
- GitHub Check: 🔬 unit-no-extras on 3.10
- GitHub Check: 🔬 integration-postgres on 3.11
- GitHub Check: 🔬 unit on 3.10
- GitHub Check: 🔬 unit-no-extras on 3.12
- GitHub Check: 🔬 unit-no-extras on 3.13
- GitHub Check: 🔬 unit on 3.11
- GitHub Check: 🔬 unit on 3.13
- GitHub Check: Optimize new code in this PR
🔇 Additional comments (33)
src/strawchemy/schema/field.py (2)
115-116: LGTM: Formatting improvement.The added blank line improves readability by visually separating the pagination logic from the id_field_name assignment.
236-237: LGTM: Formatting improvement.The added blank line improves readability within the cached property.
src/strawchemy/mapper.py (1)
290-296: LGTM: Proper integration of pagination_default_offset.The DefaultOffsetPagination is now constructed with both
limitandoffsetfrom the config whenpagination=True, correctly implementing the new pagination defaults feature.src/strawchemy/dto/utils.py (1)
22-22: LGTM: Type alias updated consistently.The import and parameter type correctly changed from
ExcludeFieldstoFieldIterable, aligning with the broader refactoring across the codebase.Also applies to: 48-48
tests/unit/mapping/test_schemas.py (1)
188-193: LGTM: Comprehensive test coverage for new pagination and order features.The added test scenarios properly cover the new pagination and order API features including default offsets, specific field pagination/ordering, empty lists, and combined scenarios.
Also applies to: 202-203
src/strawchemy/config/base.py (1)
50-57: LGTM: Well-organized configuration additions.The new configuration options are properly grouped with related pagination settings and have sensible defaults:
order_by: bool = Falsefor enabling order by on list resolverspagination_default_offset: int = 0for default pagination offsetThe docstrings are clear and follow the existing style.
tests/unit/schemas/pagination/pagination_default_offset.py (1)
1-18: LGTM: Proper test for pagination_default_offset.The test correctly exercises the new
pagination_default_offset=5configuration option with appropriate type decorators (include="all",paginate="all") and field setup (pagination=True).src/strawchemy/validation/pydantic.py (1)
27-27: LGTM: Consistent type alias update.The import and parameter type correctly updated from
ExcludeFieldstoFieldIterable, maintaining consistency with the broader refactoring effort.Also applies to: 87-87
src/strawchemy/schema/factories/inputs.py (2)
35-35: LGTM: Consistent type alias update.The import and parameter type correctly updated from
ExcludeFieldstoFieldIterable, maintaining consistency with the broader refactoring.Also applies to: 55-55
146-148: Confirmed:partial=Truefor aggregation filters is intentional and documented.The addition of
partial=Truemakes all fields in the aggregation filter DTO optional, which is the intended behavior. This is documented inDTOConfigand follows the same pattern used for aggregate types attypes.py:419. Aggregation filters are naturally optional—developers specify only the filters they need—which aligns with GraphQL filter input conventions. Existing integration tests intests/integration/test_aggregation_filters.pyvalidate this behavior across count, min/max string, sum, and avg aggregations.src/strawchemy/dto/base.py (2)
35-36: LGTM!Import of
FieldIterablealigns with the type system refactor replacingExcludeFields.
688-688: LGTM!The
excludeparameter type updated toFieldIterable | Noneis consistent with the broader API refactor.src/strawchemy/schema/factories/enum.py (1)
13-13: LGTM!Import and type annotation updates for
FieldIterableare consistent with the broader DTO type system refactor.Also applies to: 144-144, 167-167
src/strawchemy/utils/registry.py (2)
104-106: LGTM with observation on type asymmetry.The new
orderandpaginatefields properly support per-field configuration. Note thatorderacceptsLiteral["all"]whilepaginatedoes not—verify this asymmetry is intentional.
114-114: LGTM!The
scoped_idproperty correctly maintains the existing identity tuple structure.src/strawchemy/dto/types.py (4)
26-26: LGTM!The
FieldIterabletype alias is well-defined and properly exported. The updatedIncludeFieldstype correctly extendsFieldIterablewith the"all"literal.Also applies to: 33-34
163-163: LGTM!The
excludefield type updated toFieldIterableis consistent with the new type system.
222-222: LGTM!The
copy_withmethod'sexcludeparameter type correctly updated toFieldIterable | None.
298-337: LGTM!The
is_field_includedimplementation correctly handles all documented cases:
- Returns
Truewheninclude="all"and field not inexclude- Returns
Truewhen field is in specificincludeset and not inexclude- Returns
Falsewhen field is explicitly excludedThe logic
(name in self.include or self.include == "all") and name not in self.excludeproperly respects both include and exclude constraints.src/strawchemy/schema/factories/types.py (4)
23-23: LGTM!Import of
IncludeFieldsaligns with the new per-field inclusion configuration.
106-192: LGTM!The
_add_fields_argumentsmethod is well-designed:
- Clear documentation of the pagination/ordering behavior
- Properly uses
DTOConfig.from_includeto convert field specifications- Uses
is_field_includedto check per-field configuration- JSON path filtering for JSON/dict fields is a nice enhancement
One note: Line 162 uses
default_pagination or True, which means pagination defaults toTruewhendefault_paginationisNone. This seems intentional for enabling pagination without custom limits.
238-284: LGTM!The factory method properly propagates the new
order,paginate, anddefault_paginationparameters. The logic at lines 262-264 to only pass"all"values to nested types (otherwiseNone) correctly scopes the propagation of global settings.
588-602: LGTM!The
InputFactory._cache_keysimplification correctly delegates to the parent implementation while adding mode-specific cache key components.src/strawchemy/schema/factories/base.py (10)
48-55: LGTM!Import reorganization properly moves
DefaultOffsetPaginationintoTYPE_CHECKINGand adds the newFieldIterableandIncludeFieldstypes.
69-76: LGTM!The
_to_frozenset_or_boolhelper correctly convertsIncludeFieldsspecifications to thefrozenset | boolformat expected byRegistryTypeInfo. The match statement handles all cases cleanly.
103-134: LGTM!The
_type_infomethod properly integrates the neworder,paginate, anddefault_paginationparameters intoRegistryTypeInfoconstruction using the_to_frozenset_or_boolhelper.
136-162: LGTM!The
_register_typemethod correctly propagates the new pagination/ordering parameters to_type_info.
196-218: LGTM!The
_configmethod properly acceptsFieldIterablefor theexcludeparameter.
220-278: LGTM!The
_type_wrappermethod properly:
- Accepts the new
paginate,order, anddefault_paginationparameters- Falls back to
self._mapper.configdefaults when parameters areNone(lines 267-269)- Propagates these to the factory call
280-366: LGTM!Good refactoring:
_input_wrappernow delegates tomake_inputfor cleaner code reusemake_inputis a well-structured public helper that can be called directly without the decorator pattern
488-533: LGTM!The
StrawchemyMappedFactory.typemethod properly exposes the newpaginate,order, anddefault_paginationparameters in the public API and propagates them to_type_wrapper.
536-571: LGTM!The
StrawchemyMappedFactory.inputmethod properly acceptsFieldIterableforexclude.
606-684: LGTM!The
StrawchemyUnMappedDTOFactorymethods properly mirror the updates inStrawchemyMappedFactorywith the new pagination/ordering parameters andFieldIterabletype forexclude.
src/strawchemy/dto/types.py
Outdated
| @classmethod | ||
| def from_include(cls, include: IncludeFields | bool | None = None, purpose: Purpose = Purpose.READ) -> Self: | ||
| """Create a DTOConfig from an include specification. | ||
|
|
||
| Factory method for creating a DTOConfig with a simplified interface, converting | ||
| an `IncludeFields` specification into a complete configuration object. This is | ||
| useful for building configs when only the include/exclude specification matters. | ||
|
|
||
| Args: | ||
| include: The field inclusion specification. Can be: | ||
| - None: Include no fields (converted to empty set) | ||
| - "all": Include all fields | ||
| - list or set of field names: Include only these specific fields | ||
| Defaults to None. | ||
| purpose: The purpose of the DTO being configured (READ, WRITE, or COMPLETE). | ||
| Defaults to Purpose.READ. | ||
|
|
||
| Returns: | ||
| A new DTOConfig instance with the specified include and purpose settings. | ||
| All other configuration parameters use their defaults. | ||
| """ | ||
| match include: | ||
| case True: | ||
| include_ = "all" | ||
| case False | None: | ||
| include_ = set() | ||
| case _: | ||
| include_ = include | ||
| return cls(purpose, include=include_) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider making include keyword-only to avoid boolean positional argument ambiguity.
The static analysis flags include: IncludeFields | bool | None as a boolean-typed positional argument (FBT001). While functional, callers might accidentally pass a boolean in an unintended position.
🔎 Proposed fix
@classmethod
-def from_include(cls, include: IncludeFields | bool | None = None, purpose: Purpose = Purpose.READ) -> Self:
+def from_include(cls, *, include: IncludeFields | bool | None = None, purpose: Purpose = Purpose.READ) -> Self:🧰 Tools
🪛 Ruff (0.14.10)
189-189: Boolean-typed positional argument in function definition
(FBT001)
🤖 Prompt for AI Agents
In src/strawchemy/dto/types.py around lines 188 to 216, the from_include method
accepts include as a positional argument which can be misused by passing a
boolean positionally; change the signature to make include keyword-only (e.g.,
introduce a single '*' before include) so callers must pass include=<...>, keep
purpose as a normal parameter with its default, and update any call sites to use
the new keyword for include; no other logic changes required.
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 (1)
src/strawchemy/config/base.py (1)
28-29: Docstring inconsistency with new API.The docstring says
pagination=Truebut the new API usespagination="all". The docstring should be updated to reflect the new semantics.🔎 Proposed fix
- pagination_default_limit: Default pagination limit when `pagination=True`. + pagination_default_limit: Default pagination limit when pagination is enabled.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[order_config_default].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[pagination_config_default].gqlis excluded by!**/__snapshots__/**
📒 Files selected for processing (7)
src/strawchemy/config/base.pysrc/strawchemy/mapper.pysrc/strawchemy/schema/field.pysrc/strawchemy/schema/mutation/field_builder.pytests/unit/mapping/test_schemas.pytests/unit/schemas/order/order_config_default.pytests/unit/schemas/pagination/pagination_config_default.py
🧰 Additional context used
🧬 Code graph analysis (4)
src/strawchemy/config/base.py (1)
src/strawchemy/schema/field.py (1)
order_by(238-247)
src/strawchemy/schema/field.py (2)
src/strawchemy/schema/factories/inputs.py (1)
OrderByDTOFactory(346-475)src/strawchemy/dto/strawberry.py (1)
OrderByDTO(451-467)
src/strawchemy/schema/mutation/field_builder.py (1)
src/strawchemy/schema/factories/inputs.py (1)
OrderByDTOFactory(346-475)
tests/unit/schemas/pagination/pagination_config_default.py (3)
tests/fixtures.py (1)
strawchemy(34-35)src/strawchemy/mapper.py (1)
Strawchemy(61-698)src/strawchemy/config/base.py (1)
StrawchemyConfig(18-63)
⏰ 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). (20)
- GitHub Check: 🔬 integration-mysql on 3.11
- GitHub Check: 🔬 integration-sqlite on 3.13
- GitHub Check: 🔬 integration-postgres on 3.14
- GitHub Check: 🔬 integration-postgres on 3.12
- GitHub Check: 🔬 unit-no-extras on 3.10
- GitHub Check: 🔬 unit-no-extras on 3.12
- GitHub Check: 🔬 unit-no-extras on 3.11
- GitHub Check: 🔬 integration-postgres on 3.11
- GitHub Check: 🔬 integration-sqlite on 3.12
- GitHub Check: 🔬 integration-mysql on 3.12
- GitHub Check: 🔬 unit on 3.13
- GitHub Check: 🔬 unit on 3.12
- GitHub Check: 🔬 integration-postgres on 3.10
- GitHub Check: 🔬 integration-mysql on 3.10
- GitHub Check: 🔬 unit on 3.10
- GitHub Check: 🔬 unit on 3.14
- GitHub Check: 🔬 unit-no-extras on 3.13
- GitHub Check: 🔬 unit-no-extras on 3.14
- GitHub Check: 🔬 unit on 3.11
- GitHub Check: Optimize new code in this PR
🔇 Additional comments (14)
tests/unit/schemas/order/order_config_default.py (3)
1-6: LGTM!The imports are appropriate for this test schema definition.
8-8: LGTM!The configuration correctly demonstrates the new
order_by="all"global setting, aligning with the PR's refactoring objectives.
11-18: Schema definition is correct and properly integrated.The type definitions are well-structured. FruitType properly uses
@strawchemy.type(Fruit, include="all")to expose all fields, and the Query type correctly defines the fruits field usingstrawchemy.field(). This fixture file is referenced intests/unit/mapping/test_schemas.py:202as a pytest parameter ("order.order_config_default.Query"), confirming it is properly integrated into the test suite.tests/unit/mapping/test_schemas.py (2)
156-161: AI summary inconsistency detected.The AI-generated summary claims changes were made to this test function, but no modifications are visible here. All actual changes are in the
test_query_schemasparameterized test cases (lines 188-193 and 202-204).
188-193: Good expansion of test coverage for pagination and order API.The new test cases appropriately cover various pagination and ordering scenarios that align with the PR's refactoring objectives. The naming is consistent and descriptive. All referenced test schema files exist and the test structure is correct.
src/strawchemy/schema/mutation/field_builder.py (1)
18-18: LGTM!The
OrderByDTOFactoryis correctly added as a required dependency and properly propagated through the mutation field construction. The TYPE_CHECKING import pattern is appropriate for avoiding circular imports.Also applies to: 39-39, 109-109
tests/unit/schemas/pagination/pagination_config_default.py (1)
8-8: LGTM!The test correctly uses the new
pagination="all"API, which aligns with the refactoredStrawchemyConfig.paginationtype ofLiteral["all"] | None.src/strawchemy/config/base.py (1)
50-53: LGTM!The new
paginationandorder_byfields withLiteral["all"] | Nonetyping provide clear, explicit semantics for global configuration. This is a good improvement over boolean flags.src/strawchemy/schema/field.py (3)
79-81: LGTM!The addition of
order_by_factoryparameter and widening oforder_bytype totype[OrderByDTO] | bool | Noneenables both explicit type specification and boolean flag control, which aligns well with the new global configuration pattern.
237-247: LGTM!The
order_byproperty logic correctly handles all precedence cases:
- Explicit
Trueor global config enable → use factoryNonewithout global config → fall back to type's defaultFalse→ explicitly disabled- Custom type → use as-is
The implementation is clean and the precedence is intuitive.
366-366: LGTM!Correctly propagates
order_by_factorywhen copying the field, ensuring cloned fields maintain proper factory reference.src/strawchemy/mapper.py (3)
138-142: LGTM!The
MutationFieldBuilderconstruction now uses explicit keyword arguments, improving readability and correctly wiring the neworder_by_factorydependency.
176-176: LGTM!The
order_byparameter type is consistently updated across all overloads totype[OrderByDTO] | bool | None, enabling both explicit type specification and boolean flag control.Also applies to: 204-204, 232-232
332-332: LGTM!Correctly wires
order_by_factorytoStrawchemyField, completing the dependency injection chain fromStrawchemy→StrawchemyField.
| pagination_default_limit: int = 100 | ||
| """Default pagination limit when `pagination=True`.""" | ||
| pagination_default_offset: int = 0 | ||
| """Default pagination offset when `pagination=True`.""" |
There was a problem hiding this comment.
Inline docstrings reference outdated API.
The docstrings on lines 55 and 57 mention pagination=True, but the new API uses pagination="all".
🔎 Proposed fix
pagination_default_limit: int = 100
- """Default pagination limit when `pagination=True`."""
+ """Default pagination limit when pagination is enabled."""
pagination_default_offset: int = 0
- """Default pagination offset when `pagination=True`."""
+ """Default pagination offset when pagination is enabled."""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pagination_default_limit: int = 100 | |
| """Default pagination limit when `pagination=True`.""" | |
| pagination_default_offset: int = 0 | |
| """Default pagination offset when `pagination=True`.""" | |
| pagination_default_limit: int = 100 | |
| """Default pagination limit when pagination is enabled.""" | |
| pagination_default_offset: int = 0 | |
| """Default pagination offset when pagination is enabled.""" |
🤖 Prompt for AI Agents
In src/strawchemy/config/base.py around lines 54 to 57, the inline docstrings
reference the outdated API value `pagination=True`; update both docstrings to
reference the new API value `pagination="all"` (e.g., change `when
`pagination=True`` to `when `pagination="all"``) so they accurately reflect the
current parameter semantics and quoting style.
src/strawchemy/mapper.py
Outdated
| pagination = ( | ||
| DefaultOffsetPagination(limit=self.config.pagination_default_limit) if pagination is True else pagination | ||
| DefaultOffsetPagination( | ||
| limit=self.config.pagination_default_limit, offset=self.config.pagination_default_offset | ||
| ) | ||
| if pagination is True | ||
| else pagination | ||
| ) | ||
| if pagination is None: | ||
| pagination = self.config.pagination | ||
| pagination = self.config.pagination == "all" |
There was a problem hiding this comment.
Config defaults not applied when pagination is derived from global config.
When pagination=None and config.pagination == "all", line 302 sets pagination = True. However, this boolean True is passed to StrawchemyField, where it creates DefaultOffsetPagination() with its own defaults (line 114-116 in field.py), bypassing config.pagination_default_limit and config.pagination_default_offset.
The config defaults are only applied when the user explicitly passes pagination=True (lines 294-300).
🔎 Proposed fix
pagination = (
DefaultOffsetPagination(
limit=self.config.pagination_default_limit, offset=self.config.pagination_default_offset
)
if pagination is True
else pagination
)
if pagination is None:
- pagination = self.config.pagination == "all"
+ pagination = (
+ DefaultOffsetPagination(
+ limit=self.config.pagination_default_limit, offset=self.config.pagination_default_offset
+ )
+ if self.config.pagination == "all"
+ else False
+ )🤖 Prompt for AI Agents
In src/strawchemy/mapper.py around lines 294-302, the logic that converts
pagination=True into a DefaultOffsetPagination instance runs before we derive
pagination from the global config, so when pagination is None and
config.pagination == "all" we end up setting pagination=True after that
conversion and skip applying config.pagination_default_limit/offset. Move or
change the config-derived assignment so that the check "if pagination is None:
pagination = (self.config.pagination == 'all')" happens before the block that
converts a True value into DefaultOffsetPagination, ensuring the
DefaultOffsetPagination is constructed using
self.config.pagination_default_limit and pagination_default_offset.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
README.mdsrc/strawchemy/dto/types.pysrc/strawchemy/schema/factories/base.py
🧰 Additional context used
🧬 Code graph analysis (2)
src/strawchemy/dto/types.py (2)
src/strawchemy/dto/utils.py (1)
field(72-83)src/strawchemy/dto/base.py (1)
name(318-319)
src/strawchemy/schema/factories/base.py (5)
src/strawchemy/dto/types.py (2)
DTOConfig(115-311)Purpose(67-87)src/strawchemy/schema/pagination.py (1)
DefaultOffsetPagination(9-11)src/strawchemy/dto/utils.py (1)
config(45-69)src/strawchemy/schema/factories/inputs.py (2)
factory(166-193)factory(446-475)src/strawchemy/schema/factories/types.py (3)
factory(226-284)factory(348-377)factory(691-720)
🪛 LanguageTool
README.md
[grammar] ~2105-~2105: Use a hyphen to join words.
Context: ... | Automatically convert snake cased names to camel case in GraphQL sch...
(QB_NEW_EN_HYPHEN)
🪛 Ruff (0.14.10)
src/strawchemy/schema/factories/base.py
340-340: Dynamically typed expressions (typing.Any) are disallowed in **kwargs
(ANN401)
647-647: Boolean-typed positional argument in function definition
(FBT001)
⏰ 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). (20)
- GitHub Check: 🔬 integration-mysql on 3.11
- GitHub Check: 🔬 integration-postgres on 3.12
- GitHub Check: 🔬 integration-mysql on 3.13
- GitHub Check: 🔬 integration-sqlite on 3.12
- GitHub Check: 🔬 unit-no-extras on 3.10
- GitHub Check: 🔬 integration-sqlite on 3.14
- GitHub Check: 🔬 integration-sqlite on 3.11
- GitHub Check: 🔬 integration-postgres on 3.13
- GitHub Check: 🔬 unit-no-extras on 3.13
- GitHub Check: 🔬 integration-postgres on 3.11
- GitHub Check: 🔬 integration-postgres on 3.14
- GitHub Check: 🔬 unit-no-extras on 3.12
- GitHub Check: 🔬 integration-mysql on 3.10
- GitHub Check: 🔬 integration-sqlite on 3.13
- GitHub Check: 🔬 unit on 3.14
- GitHub Check: 🔬 integration-postgres on 3.10
- GitHub Check: 🔬 unit on 3.11
- GitHub Check: 🔬 unit on 3.10
- GitHub Check: 🔬 unit on 3.12
- GitHub Check: Optimize new code in this PR
🔇 Additional comments (11)
src/strawchemy/dto/types.py (3)
10-10: LGTM: Import ofSelfis appropriate.The
Selftype fromtyping_extensionsis correctly imported for use in thefrom_includeclassmethod return type annotation.
33-34: LGTM: Type aliases enhance flexibility.The new
FieldIterabletype alias broadens the acceptable field collection types (addingfrozensetandtuple), andIncludeFieldsprovides a clear semantic distinction for inclusion specifications.
292-311: LGTM: Field inclusion logic correctly respects exclude.The implementation correctly checks both inclusion and exclusion rules. When
include == "all", it properly respects theexcludeset by returningFalsefor excluded fields. This addresses the issue raised in previous reviews.src/strawchemy/schema/factories/base.py (5)
103-134: LGTM: Pagination and ordering parameters properly threaded through type registration.The
_type_infomethod correctly accepts and processes the neworder,paginate, anddefault_paginationparameters, using the helper function to normalize them before passing toRegistryTypeInfo.
321-367: LGTM: Newmake_inputmethod provides useful programmatic API.The
make_inputmethod is a well-structured addition that enables programmatic input DTO creation without the decorator pattern. The use of**kwargs(flagged by static analysis as ANN401) is an acceptable pattern in factory methods for extensibility.
220-278: LGTM: Configuration cascade correctly implemented.The
_type_wrapperproperly implements a configuration cascade where field-level parameters (paginate,order,default_pagination) override config-level defaults. This provides flexibility while maintaining sensible defaults.
488-571: LGTM: Public method signatures consistently updated.The
type()andinput()decorator methods have been properly updated with the new pagination and ordering parameters. The signatures are consistent with the internal factory changes and maintain backward compatibility with default values.
103-162: LGTM: Refactoring is systematic and consistent.The pagination and ordering parameter changes are applied consistently across all factory methods (
_type_info,_register_type,_type_wrapper,_input_wrapper,type,input). The transition fromExcludeFieldstoFieldIterableis also uniformly applied throughout.Also applies to: 220-367, 488-571
README.md (3)
571-656: LGTM: Pagination documentation is comprehensive and accurate.The expanded pagination section clearly documents three usage levels (field, config, type) with accurate code examples. The distinction between
pagination(config/field level) andpaginate(type decorator) parameter names is correctly reflected.
658-768: LGTM: Comprehensive ordering documentation added.The new Ordering section provides clear examples across all configuration levels (field, type, config) and demonstrates nested relationship ordering. The examples accurately reflect the new API with
orderandorder_byparameters.
2128-2139: LGTM: Configuration example demonstrates new features correctly.The example accurately shows how to configure global pagination and ordering using
pagination="all"andorder_by="all"along with default pagination limits and offsets.
| |-----------------------------|-------------------------------------------------------------|----------------------------|------------------------------------------------------------------------------------------------------------------------------------------| | ||
| | `dialect` | `SupportedDialect` | | Database dialect to use. Supported dialects are "postgresql", "mysql", "sqlite". | | ||
| | `session_getter` | `Callable[[Info], Session]` | `default_session_getter` | Function to retrieve SQLAlchemy session from strawberry `Info` object. By default, it retrieves the session from `info.context.session`. | | ||
| | `auto_snake_case` | `bool` | `True` | Automatically convert snake cased names to camel case in GraphQL schema. | |
There was a problem hiding this comment.
Fix hyphenation in configuration table.
"snake cased names" should be hyphenated as "snake-cased names" for proper English grammar.
🔎 Proposed fix
-| `auto_snake_case` | `bool` | `True` | Automatically convert snake cased names to camel case in GraphQL schema. |
+| `auto_snake_case` | `bool` | `True` | Automatically convert snake-cased names to camel case in GraphQL schema. |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | `auto_snake_case` | `bool` | `True` | Automatically convert snake cased names to camel case in GraphQL schema. | | |
| | `auto_snake_case` | `bool` | `True` | Automatically convert snake-cased names to camel case in GraphQL schema. | |
🧰 Tools
🪛 LanguageTool
[grammar] ~2105-~2105: Use a hyphen to join words.
Context: ... | Automatically convert snake cased names to camel case in GraphQL sch...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
In README.md around line 2105, the phrase "snake cased names" in the
configuration table should be corrected to "snake-cased names"; update the table
cell text to use the hyphenated form "snake-cased names" so the grammar is
correct while preserving the rest of the sentence and formatting.
| @classmethod | ||
| def from_include(cls, include: IncludeFields | None = None, purpose: Purpose = Purpose.READ) -> Self: | ||
| """Create a DTOConfig from an include specification. | ||
|
|
||
| Factory method for creating a DTOConfig with a simplified interface, converting | ||
| an `IncludeFields` specification into a complete configuration object. This is | ||
| useful for building configs when only the include/exclude specification matters. | ||
|
|
||
| Args: | ||
| include: The field inclusion specification. Can be: | ||
| - None: Include no fields (converted to empty set) | ||
| - "all": Include all fields | ||
| - list or set of field names: Include only these specific fields | ||
| Defaults to None. | ||
| purpose: The purpose of the DTO being configured (READ, WRITE, or COMPLETE). | ||
| Defaults to Purpose.READ. | ||
|
|
||
| Returns: | ||
| A new DTOConfig instance with the specified include and purpose settings. | ||
| All other configuration parameters use their defaults. | ||
| """ | ||
| return cls(purpose, include=set() if include is None else include) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider the past suggestion to make include keyword-only.
A previous review recommended making the include parameter keyword-only to avoid potential misuse with boolean positional arguments. While the current implementation is functional, the suggestion remains valid for improved API safety.
🔎 Reference to past review
The past review suggested:
@classmethod
def from_include(cls, *, include: IncludeFields | None = None, purpose: Purpose = Purpose.READ) -> Self:This would require callers to explicitly use include=... when calling the method.
🤖 Prompt for AI Agents
In src/strawchemy/dto/types.py around lines 188 to 209, change the classmethod
signature to make include a keyword-only parameter to prevent accidental
positional misuse: update the signature to accept a leading "*" (e.g., def
from_include(cls, *, include: IncludeFields | None = None, purpose: Purpose =
Purpose.READ) -> Self), keep the existing logic that converts include None to an
empty set, and ensure any call sites are updated to pass include=... (and
purpose=... if needed).
| def _include_fields_to_set_or_bool(value: IncludeFields | None) -> frozenset[Any] | bool: | ||
| match value: | ||
| case None: | ||
| return frozenset() | ||
| case "all": | ||
| return True | ||
| case _: | ||
| return frozenset(value) | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding return type annotation.
The helper function _include_fields_to_set_or_bool is well-implemented with clear logic. However, adding a return type annotation would improve type safety and code documentation.
🔎 Proposed enhancement
-def _include_fields_to_set_or_bool(value: IncludeFields | None) -> frozenset[Any] | bool:
+def _include_fields_to_set_or_bool(value: IncludeFields | None) -> frozenset[str] | bool:Note: The return type is already present, but could be more specific with frozenset[str] instead of frozenset[Any].
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _include_fields_to_set_or_bool(value: IncludeFields | None) -> frozenset[Any] | bool: | |
| match value: | |
| case None: | |
| return frozenset() | |
| case "all": | |
| return True | |
| case _: | |
| return frozenset(value) | |
| def _include_fields_to_set_or_bool(value: IncludeFields | None) -> frozenset[str] | bool: | |
| match value: | |
| case None: | |
| return frozenset() | |
| case "all": | |
| return True | |
| case _: | |
| return frozenset(value) |
🤖 Prompt for AI Agents
In src/strawchemy/schema/factories/base.py around lines 69 to 77, the function
_include_fields_to_set_or_bool currently annotates its return as frozenset[Any]
| bool; change this to a more specific frozenset[str] | bool to improve type
safety and documentation. Update the signature return type to frozenset[str] |
bool and ensure any conversion to frozenset(value) treats value as an iterable
of str (e.g., cast or rely on IncludeFields being Iterable[str]); add or adjust
imports/forward refs if needed so the new annotation resolves correctly.
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/strawchemy/schema/factories/inputs.py (1)
166-194: Missingno_cacheparameter forwarding in_FilterDTOFactory.factory.The
no_cacheparameter is added to the method signature (line 177) but is not forwarded tosuper().factory()call (lines 182-193). This means the cache bypass won't work for_FilterDTOFactorysubclasses.🐛 Proposed fix
def factory( self, model: type[DeclarativeT], dto_config: DTOConfig, base: type[Any] | None = None, name: str | None = None, parent_field_def: DTOFieldDefinition[DeclarativeBase, QueryableAttribute[Any]] | None = None, current_node: Node[Relation[Any, GraphQLFilterDTOT], None] | None = None, raise_if_no_fields: bool = False, tags: set[str] | None = None, backend_kwargs: dict[str, Any] | None = None, no_cache: bool = False, *, aggregate_filters: bool = True, **kwargs: Any, ) -> type[GraphQLFilterDTOT]: return super().factory( model, dto_config, base, name, parent_field_def, current_node, raise_if_no_fields, tags, backend_kwargs, + no_cache, aggregate_filters=aggregate_filters, **kwargs, )src/strawchemy/schema/field.py (1)
420-451:__copy__loses explicitpagination=Falseand may re-enable pagination via config.
Line 444 passespagination=self.pagination, butself.paginationreturnsNonewhen_paginationisFalse, so the copied field will behave as if pagination was never explicitly disabled (and may inherit"all"from config).Proposed fix
@@ def __copy__(self) -> Self: new_field = type(self)( @@ - pagination=self.pagination, + pagination=self._pagination, registry_namespace=self.registry_namespace, execution_options=self._execution_options, config=self._config, order_by_factory=self._order_by_factory, filter_factory=self._filter_factory, distinct_on_factory=self._distinct_on_factory, )src/strawchemy/schema/factories/base.py (3)
410-461:no_cacheis accepted but not forwarded inStrawchemyMappedFactory.factory()(breaksmake_input(..., no_cache=True)).
GraphQLDTOFactory.make_input()passesno_cache=...intoself.factory(...), butStrawchemyMappedFactory.factory()drops it when callingsuper().factory(...). That can re-enable caching / ignore uniquified names, causing type collisions or wrong reuse.Proposed fix
@@ class StrawchemyMappedFactory(GraphQLDTOFactory[MappedGraphQLDTOT]): @@ def factory( @@ - no_cache: bool = False, + no_cache: bool = False, *, mode: GraphQLPurpose | None = None, **kwargs: Any, ) -> type[MappedGraphQLDTOT]: @@ return super().factory( model, dto_config, base, name, parent_field_def, current_node, raise_if_no_fields, tags, backend_kwargs=backend_kwargs, + no_cache=no_cache, mode=mode, **kwargs, )Also applies to: 579-610
648-689: Ruff FBT001: make bool params keyword-only inStrawchemyUnMappedDTOFactory.type()by adding*.
This also brings it in line withStrawchemyMappedFactory.type()which is already keyword-only.Proposed fix
@@ class StrawchemyUnMappedDTOFactory(GraphQLDTOFactory[UnmappedGraphQLDTOT]): @@ def type( self, model: type[T], - include: IncludeFields | None = None, + *, + include: IncludeFields | None = None, exclude: FieldIterable | None = None, partial: bool | None = None, type_map: Mapping[Any, Any] | None = None, aliases: Mapping[str, str] | None = None, alias_generator: Callable[[str], str] | None = None, paginate: IncludeFields | None = None, default_pagination: None | DefaultOffsetPagination = None, filter_input: type[BooleanFilterDTO] | None = None, order_by: IncludeFields | type[OrderByDTO] | None = None, name: str | None = None, description: str | None = None, directives: Sequence[object] | None = (), query_hook: QueryHook[T] | list[QueryHook[T]] | None = None, override: bool = False, purpose: Purpose = Purpose.READ, mode: GraphQLPurpose = "type", ) -> Callable[[type[Any]], type[UnmappedStrawberryGraphQLDTO[T]]]:
95-123: Fix the silent conversion ofNonetofrozenset()fororder,distinct_on, andpaginatefields inRegistryTypeInfo.The current logic converts
Noneto an emptyfrozenset()becauseis_fields_iterable(None)returnsTrue. This breaks the semantic distinction between "not specified" (None) and "explicitly empty" (frozenset()). Additionally, theRegistryTypeInfofield type annotations do not acceptNone—they only acceptfrozenset[str] | Literal["all"] | type[OrderByDTO/EnumDTO]—creating a type mismatch between the method parameters and the dataclass fields.Either:
- Exclude
Nonefromis_fields_iterable()and handle it separately in_type_info(), or- Explicitly document that
Noneparameters are normalized to emptyfrozenset()and update type annotations to reflect this behavior.
🤖 Fix all issues with AI agents
In @examples/testapp/testapp/types.py:
- Line 23: The project mixes two ordering parameters: the type decorator uses
order= (e.g., @strawchemy.type(Ticket, include="all", filter_input=TicketFilter,
order=TicketOrder, override=True)) while field-level calls still use order_by=;
update all strawchemy.field(...) usages to replace order_by= with order= so the
field-level API matches the type-level API (search for strawchemy.field in tests
and examples, e.g., tests/unit/schemas/order/field_order_by_specific_fields.py
and examples/testapp/testapp/types.py references to
Ticket/TicketFilter/TicketOrder).
In @src/strawchemy/config/base.py:
- Around line 51-64: Update the field docstrings to correct their descriptions:
change the docstring for the exclude field (symbol: exclude: FieldIterable |
None) from "Globally included fields." to something like "Globally excluded
fields." and fix the typo in the distinct_on field docstring (symbol:
distinct_on: IncludeFields | None) from "Enable/disable order by onelist
resolvers." to "Enable/disable order by on list resolvers."
In @src/strawchemy/dto/strawberry.py:
- Around line 132-134: DTOKey is missing a __slots__ declaration so instances
still get per-instance __dict__ despite inheriting from _Key; add a class-level
__slots__ = () to the DTOKey class (the DTOKey class definition referenced in
the diff) to prevent per-instance dictionaries and complete the memory
optimization already begun by _Key and the separator ClassVar.
In @src/strawchemy/mapper.py:
- Around line 86-87: Tidy the type annotation namespaces by removing dunder
entries before performing the dict unions: when building TYPING_NS in
dto/base.py (symbol TYPING_NS) filter vars(typing) and vars(types) to exclude
keys that start and end with "__" and use those cleaned dicts for the union;
similarly in mapper.py (symbol _types_namespace) filter vars(mutation_types) to
remove dunder keys and also ensure you union against the already-cleaned
TYPING_NS so both sources are consistently dunder-free.
In @src/strawchemy/schema/factories/base.py:
- Around line 301-373: The make_input signature currently uses an untyped
**kwargs: Any which triggers Ruff ANN401; change the var-keyword param to
**kwargs: object (or to a concrete Unpack[...] type if you have a stable shape)
in the make_input definition and update any related type annotations that depend
on it, and keep the forwarding call to self.factory(model=..., **kwargs) as-is;
specifically edit the make_input function's parameter list (the **kwargs: Any
token) and ensure usages like the call to self.factory remain compatible with
the new **kwargs: object typing.
In @tests/unit/schemas/distinct/distinct_config_all.py:
- Around line 1-18: Add a module-level docstring at the top describing that
distinct_on is PostgreSQL-specific and that configuring StrawchemyConfig(...,
distinct_on="all") enables PostgreSQL's DISTINCT ON semantics for all queries;
reference the StrawchemyConfig instantiation and the Strawchemy type usage
(strawchemy = Strawchemy(StrawchemyConfig("postgresql", distinct_on="all")) and
@strawchemy.type(Color, include="all")) so readers know the scope and expected
behavior when distinct_on="all" is set globally.
In @tests/unit/schemas/distinct/distinct_config_empty.py:
- Line 8: Add a module-level docstring to
tests/unit/schemas/distinct/distinct_config_empty.py explaining that passing
distinct_on=[] intentionally disables distinct_on behavior because empty lists
are falsy in Python; reference that this matches the runtime check in field.py
where `if self._is_strawchemy_type(inner_type) and distinct_on:` treats `[]`
equivalently to `None`, so the test demonstrates the disabled behavior. Ensure
the docstring clearly states intent for future readers and mentions the relevant
symbols `Strawchemy`, `StrawchemyConfig(distinct_on=[])`, and the
`self._is_strawchemy_type` + `distinct_on` check in `field.py`.
In @tests/unit/schemas/distinct/distinct_config_specific_fields.py:
- Around line 11-18: Add a short module-level docstring or a top-of-file comment
explaining that this test demonstrates config-level distinct_on inheritance:
when using the decorator on ColorType (@strawchemy.type(Color, include="all"))
the distinct_on setting defined in the schema/config is applied to the generated
type and therefore affects the Query.colors field; place the comment above the
class definitions (or directly above ColorType) and mention the symbols
ColorType, Query, and distinct_on to make the implicit inheritance behavior
explicit for future readers.
In @tests/unit/schemas/distinct/field_distinct_all.py:
- Around line 11-13: Add a concise docstring to clarify test intent: explain
that this module (or the ColorType class) verifies field-level distinct_on
behavior when using @strawchemy.type(Color, include="all") and that the test
ensures distinct_on="all" is applied to fields; place the docstring at the top
of the module or directly above the ColorType class to document purpose and
expected behavior.
In @tests/unit/schemas/distinct/field_distinct_specific_fields.py:
- Around line 1-18: Add a short module-level docstring at the top of this test
file that explains its purpose: that it verifies field-level distinct_on
behavior when specific fields are provided (e.g., using
strawchemy.field(distinct_on=["name", "hex"]) on the Query.colors field). Place
the docstring above the imports (or immediately after any future __future__
import) and mention the relation to order_config_all.py for consistency.
- Around line 1-18: The test references a non-existent "hex" attribute in the
Color model via the distinct_on=["name", "hex"] argument (see Query.colors using
strawchemy.field); fix by either adding a hex: Mapped[str] field to the Color
model class (so Color has name and hex attributes used by Strawchemy/ColorType)
or by removing "hex" from the distinct_on list in the test so it only uses
existing fields (e.g., distinct_on=["name"]); update the Color model or the
distinct_on usage accordingly to keep names consistent with the Color class and
the ColorType mapping.
In @tests/unit/schemas/order/field_order_by_specific_fields.py:
- Around line 1-18: Add a concise module-level docstring at the top of the test
file describing that this test verifies field-level order_by with specific
fields on relationship/collection fields; reference the testing intent alongside
the key items in the file (the Strawchemy instance "strawchemy", the annotated
type "ContainerType", and the "Query" class that uses
strawchemy.field(order_by=[...])) so readers understand the test purpose without
reading the implementation.
In @tests/unit/schemas/order/order_config_all.py:
- Around line 1-18: Add a module-level docstring at the top of the test file
describing that this module verifies global order_by configuration when
StrawchemyConfig is created with order_by="all" and that FruitType (created via
strawchemy.type on Fruit with include="all") is used in the Query to generate
fields; place a short explanatory string before the imports explaining the
purpose of the test and mentioning the key symbols
StrawchemyConfig("postgresql", order_by="all"), strawchemy, FruitType, and the
Query.fruits field so readers understand the intent of the test.
In @tests/unit/schemas/order/order_config_empty_with_empty_type_override.py:
- Around line 11-13: Add a docstring to the ContainerType class describing the
test scenario: that it verifies behavior when a schema uses include="all" on
Container while a type-level ordering override is explicitly empty (order=[]),
clarifying expected outcome and any edge conditions; place the docstring
immediately under the class declaration for ContainerType and mention the
relationship to Container, include="all", and the empty order override so future
maintainers understand the intent.
In @tests/unit/schemas/order/order_config_specific_fields_with_type_override.py:
- Around line 1-18: Add a top-of-file module docstring explaining that this
schema demonstrates selective field ordering at the type level: ContainerType
(created via Strawchemy("postgresql") and annotated with @strawchemy.type(...,
include="all", order=["fruits", "vegetables"])) only exposes ordering for the
specified relationship fields ("fruits" and "vegetables") while other
relationships like "colors" do not support ordering; ensure the docstring
briefly states the purpose and how to interpret the Query.container field
populated via strawchemy.field().
In @tests/unit/schemas/order/order_config_specific_fields.py:
- Around line 11-18: Add a short explanatory comment or docstring above the test
types to document how config-level order_by inheritance works: mention that the
@strawchemy.type(Fruit, include="all") applied on FruitType inherits ordering
config from Fruit, and that Query.fruits uses FruitType so the test validates
that inherited order_by rules apply; place the note near the FruitType or Query
declaration referencing the symbols Fruit, FruitType, and Query so future
readers understand the intended behavior.
In @tests/unit/schemas/order/order_config_with_field_override.py:
- Around line 1-18: Add a short module-level docstring at the top of this test
describing its intent: explain that the global StrawchemyConfig is set with
order_by=["name"] but the Query.fruits field uses a field-level override
order_by=["sweetness"], and the test verifies that the field-level `order_by` on
strawchemy.field takes precedence; place this docstring above the imports so it
documents the module, mentioning the key symbols StrawchemyConfig, strawchemy,
FruitType, and Query for clarity.
In @tests/unit/schemas/pagination/pagination_config_with_type_override.py:
- Around line 11-18: Add a brief doc note explaining that type-level paginate
overrides config-level pagination using this test as an example: mention that
the @strawchemy.type annotation on Container with paginate=["vegetables"] takes
precedence over any global/config setting like pagination=["fruits"], and point
to the ContainerType class and the Query.container field in the test as the
concrete example demonstrating this precedence; keep the note short, state the
precedence rule clearly, and include the exact symbol names paginate and
pagination so readers can map behavior to code.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (24)
tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[auto_order_by].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[distinct_config_all].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[distinct_config_empty].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[distinct_config_specific_fields].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[distinct_config_with_field_override].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[field_distinct_all].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[field_distinct_specific_fields].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[field_filter_auto_generate].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[field_order_by_all].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[field_order_by_specific_fields].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[order_config_all].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[order_config_all_with_field_override].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[order_config_empty].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[order_config_empty_with_empty_type_override].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[order_config_specific_fields].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[order_config_specific_fields_with_type_override].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[order_config_with_field_override].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[paginate_empty].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[pagination_config_empty].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[pagination_config_empty_with_type_override].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[pagination_config_specific_fields].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[pagination_config_with_type_override].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[type_distinct_manual_enum].gqlis excluded by!**/__snapshots__/**tests/unit/mapping/__snapshots__/test_schemas/test_query_schemas[type_order_by_specific_fields].gqlis excluded by!**/__snapshots__/**
📒 Files selected for processing (44)
examples/testapp/testapp/types.pysrc/strawchemy/config/base.pysrc/strawchemy/dto/base.pysrc/strawchemy/dto/strawberry.pysrc/strawchemy/dto/types.pysrc/strawchemy/dto/utils.pysrc/strawchemy/mapper.pysrc/strawchemy/schema/factories/aggregations.pysrc/strawchemy/schema/factories/base.pysrc/strawchemy/schema/factories/enum.pysrc/strawchemy/schema/factories/inputs.pysrc/strawchemy/schema/factories/types.pysrc/strawchemy/schema/field.pysrc/strawchemy/schema/mutation/field_builder.pysrc/strawchemy/utils/registry.pysrc/strawchemy/validation/pydantic.pytests/unit/dto/test_dto.pytests/unit/mapping/test_schemas.pytests/unit/schemas/distinct/__init__.pytests/unit/schemas/distinct/distinct_config_all.pytests/unit/schemas/distinct/distinct_config_empty.pytests/unit/schemas/distinct/distinct_config_specific_fields.pytests/unit/schemas/distinct/distinct_config_with_field_override.pytests/unit/schemas/distinct/field_distinct_all.pytests/unit/schemas/distinct/field_distinct_specific_fields.pytests/unit/schemas/distinct/type_distinct_manual_enum.pytests/unit/schemas/filters/field_filter_auto_generate.pytests/unit/schemas/order/auto_order_by.pytests/unit/schemas/order/field_order_by_all.pytests/unit/schemas/order/field_order_by_specific_fields.pytests/unit/schemas/order/order_config_all.pytests/unit/schemas/order/order_config_all_with_field_override.pytests/unit/schemas/order/order_config_empty.pytests/unit/schemas/order/order_config_empty_with_empty_type_override.pytests/unit/schemas/order/order_config_specific_fields.pytests/unit/schemas/order/order_config_specific_fields_with_type_override.pytests/unit/schemas/order/order_config_with_field_override.pytests/unit/schemas/order/type_order_by.pytests/unit/schemas/order/type_order_by_specific_fields.pytests/unit/schemas/pagination/paginate_empty.pytests/unit/schemas/pagination/pagination_config_empty.pytests/unit/schemas/pagination/pagination_config_empty_with_type_override.pytests/unit/schemas/pagination/pagination_config_specific_fields.pytests/unit/schemas/pagination/pagination_config_with_type_override.py
💤 Files with no reviewable changes (1)
- tests/unit/schemas/order/auto_order_by.py
🧰 Additional context used
🧬 Code graph analysis (33)
tests/unit/dto/test_dto.py (1)
src/strawchemy/dto/types.py (4)
DTOConfig(117-332)from_include(200-220)Purpose(69-89)is_field_included(312-332)
tests/unit/schemas/order/type_order_by_specific_fields.py (3)
tests/unit/schemas/order/type_order_by.py (2)
SQLDataTypesType(16-16)Query(20-21)tests/unit/schemas/order/order_config_specific_fields.py (1)
Query(17-18)tests/unit/schemas/order/field_order_by_specific_fields.py (1)
Query(17-18)
tests/unit/schemas/distinct/field_distinct_all.py (2)
src/strawchemy/mapper.py (4)
Strawchemy(61-691)field(174-200)field(203-228)field(230-327)src/strawchemy/schema/field.py (1)
distinct_on(248-269)
tests/unit/schemas/order/type_order_by.py (1)
tests/unit/schemas/order/field_order_by.py (1)
SQLDataTypesOrderBy(12-12)
tests/unit/schemas/order/order_config_with_field_override.py (3)
tests/fixtures.py (1)
strawchemy(34-35)src/strawchemy/config/base.py (1)
StrawchemyConfig(19-70)src/strawchemy/schema/field.py (1)
order_by(272-301)
tests/unit/schemas/distinct/distinct_config_specific_fields.py (3)
tests/fixtures.py (1)
strawchemy(34-35)src/strawchemy/config/base.py (1)
StrawchemyConfig(19-70)src/strawchemy/schema/field.py (1)
distinct_on(248-269)
tests/unit/schemas/distinct/distinct_config_with_field_override.py (2)
src/strawchemy/config/base.py (1)
StrawchemyConfig(19-70)src/strawchemy/schema/field.py (1)
distinct_on(248-269)
tests/unit/mapping/test_schemas.py (1)
tests/integration/models.py (1)
id(61-67)
src/strawchemy/dto/utils.py (1)
tests/integration/fixtures.py (1)
config(742-746)
src/strawchemy/schema/mutation/field_builder.py (2)
src/strawchemy/schema/factories/types.py (1)
DistinctOnFieldsDTOFactory(457-462)src/strawchemy/schema/factories/inputs.py (2)
BooleanFilterDTOFactory(197-214)OrderByDTOFactory(347-478)
tests/unit/schemas/order/order_config_empty_with_empty_type_override.py (1)
tests/unit/schemas/order/order_config_specific_fields_with_type_override.py (2)
ContainerType(12-13)Query(17-18)
src/strawchemy/schema/factories/types.py (1)
src/strawchemy/dto/types.py (5)
DTOConfig(117-332)Purpose(69-89)is_fields_iterable(353-359)from_include(200-220)is_field_included(312-332)
src/strawchemy/validation/pydantic.py (1)
src/strawchemy/dto/types.py (2)
DTOConfig(117-332)Purpose(69-89)
tests/unit/schemas/distinct/distinct_config_all.py (2)
src/strawchemy/config/base.py (1)
StrawchemyConfig(19-70)src/strawchemy/schema/field.py (1)
distinct_on(248-269)
tests/unit/schemas/order/order_config_all_with_field_override.py (3)
tests/fixtures.py (1)
strawchemy(34-35)src/strawchemy/schema/field.py (1)
order_by(272-301)src/strawchemy/dto/utils.py (1)
field(78-89)
examples/testapp/testapp/types.py (3)
tests/fixtures.py (1)
strawchemy(34-35)examples/testapp/testapp/models.py (2)
Ticket(30-35)Project(52-62)src/strawchemy/transpiler/_query.py (1)
order(100-102)
src/strawchemy/schema/factories/inputs.py (2)
src/strawchemy/schema/factories/types.py (1)
_aggregation_field(90-105)src/strawchemy/dto/types.py (1)
copy_with(222-271)
tests/unit/schemas/pagination/pagination_config_specific_fields.py (4)
tests/fixtures.py (1)
strawchemy(34-35)src/strawchemy/config/base.py (1)
StrawchemyConfig(19-70)tests/unit/models.py (1)
Container(200-212)src/strawchemy/schema/field.py (1)
pagination(239-245)
src/strawchemy/dto/base.py (2)
src/strawchemy/dto/utils.py (1)
field(78-89)src/strawchemy/dto/types.py (1)
Purpose(69-89)
src/strawchemy/config/base.py (2)
tests/fixtures.py (1)
strawchemy(34-35)src/strawchemy/schema/field.py (3)
pagination(239-245)order_by(272-301)distinct_on(248-269)
tests/unit/schemas/filters/field_filter_auto_generate.py (1)
src/strawchemy/mapper.py (4)
Strawchemy(61-691)field(174-200)field(203-228)field(230-327)
src/strawchemy/utils/registry.py (2)
src/strawchemy/dto/strawberry.py (2)
EnumDTO(423-427)OrderByDTO(453-469)src/strawchemy/schema/pagination.py (1)
DefaultOffsetPagination(9-11)
tests/unit/schemas/distinct/field_distinct_specific_fields.py (3)
tests/unit/schemas/distinct/distinct_config_all.py (2)
ColorType(12-13)Query(17-18)tests/unit/schemas/distinct/distinct_config_with_field_override.py (2)
ColorType(12-13)Query(17-18)src/strawchemy/schema/field.py (1)
distinct_on(248-269)
tests/unit/schemas/order/field_order_by_all.py (1)
src/strawchemy/schema/field.py (1)
order_by(272-301)
tests/unit/schemas/distinct/distinct_config_empty.py (2)
src/strawchemy/config/base.py (1)
StrawchemyConfig(19-70)src/strawchemy/schema/field.py (1)
distinct_on(248-269)
src/strawchemy/schema/field.py (7)
src/strawchemy/dto/strawberry.py (5)
BooleanFilterDTO(472-499)EnumDTO(423-427)MappedStrawberryGraphQLDTO(430-430)OrderByDTO(453-469)StrawchemyDTOAttributes(104-111)src/strawchemy/dto/types.py (2)
DTOConfig(117-332)Purpose(69-89)src/strawchemy/schema/factories/types.py (2)
DistinctOnFieldsDTOFactory(457-462)_description(598-605)src/strawchemy/schema/factories/inputs.py (1)
BooleanFilterDTOFactory(197-214)src/strawchemy/schema/pagination.py (1)
DefaultOffsetPagination(9-11)src/strawchemy/dto/utils.py (1)
config(45-75)src/strawchemy/schema/factories/base.py (1)
_config(192-216)
tests/unit/schemas/order/field_order_by_specific_fields.py (1)
src/strawchemy/schema/field.py (1)
order_by(272-301)
tests/unit/schemas/order/order_config_all.py (4)
tests/fixtures.py (1)
strawchemy(34-35)src/strawchemy/mapper.py (4)
Strawchemy(61-691)field(174-200)field(203-228)field(230-327)src/strawchemy/config/base.py (1)
StrawchemyConfig(19-70)src/strawchemy/schema/field.py (1)
order_by(272-301)
tests/unit/schemas/pagination/pagination_config_with_type_override.py (4)
tests/fixtures.py (1)
strawchemy(34-35)tests/unit/models.py (1)
Container(200-212)src/strawchemy/schema/field.py (1)
pagination(239-245)tests/unit/schemas/pagination/pagination_config_empty_with_type_override.py (2)
ContainerType(12-13)Query(17-18)
src/strawchemy/dto/types.py (1)
src/strawchemy/dto/utils.py (1)
field(78-89)
tests/unit/schemas/order/order_config_specific_fields_with_type_override.py (6)
tests/fixtures.py (1)
strawchemy(34-35)tests/unit/models.py (1)
Container(200-212)tests/unit/schemas/order/order_config_all_with_field_override.py (1)
Query(17-18)tests/unit/schemas/order/order_config_empty.py (1)
Query(17-18)tests/unit/schemas/order/order_config_specific_fields.py (1)
Query(17-18)tests/unit/schemas/order/order_config_with_field_override.py (1)
Query(17-18)
src/strawchemy/mapper.py (3)
src/strawchemy/schema/pagination.py (1)
DefaultOffsetPagination(9-11)src/strawchemy/schema/mutation/field_builder.py (1)
MutationFieldBuilder(30-116)src/strawchemy/utils/registry.py (1)
namespace(339-340)
src/strawchemy/schema/factories/base.py (2)
src/strawchemy/dto/types.py (5)
cast_include_fields(336-336)cast_include_fields(340-340)cast_include_fields(343-350)is_fields_iterable(353-359)DTOConfig(117-332)src/strawchemy/schema/pagination.py (1)
DefaultOffsetPagination(9-11)
🪛 Ruff (0.14.10)
src/strawchemy/schema/factories/enum.py
169-169: Boolean-typed positional argument in function definition
(FBT001)
175-175: Boolean-typed positional argument in function definition
(FBT001)
175-175: Boolean default positional argument in function definition
(FBT002)
176-176: Dynamically typed expressions (typing.Any) are disallowed in **kwargs
(ANN401)
src/strawchemy/dto/base.py
638-638: Boolean-typed positional argument in function definition
(FBT001)
638-638: Boolean default positional argument in function definition
(FBT002)
src/strawchemy/dto/types.py
353-353: Dynamically typed expressions (typing.Any) are disallowed in value
(ANN401)
src/strawchemy/schema/factories/base.py
342-342: Dynamically typed expressions (typing.Any) are disallowed in **kwargs
(ANN401)
654-654: Boolean-typed positional argument in function definition
(FBT001)
⏰ 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). (20)
- GitHub Check: 🔬 integration-sqlite on 3.10
- GitHub Check: 🔬 integration-postgres on 3.13
- GitHub Check: 🔬 integration-mysql on 3.13
- GitHub Check: 🔬 integration-sqlite on 3.12
- GitHub Check: 🔬 integration-mysql on 3.12
- GitHub Check: 🔬 unit-no-extras on 3.14
- GitHub Check: 🔬 integration-sqlite on 3.13
- GitHub Check: 🔬 unit-no-extras on 3.12
- GitHub Check: 🔬 integration-mysql on 3.10
- GitHub Check: 🔬 integration-mysql on 3.11
- GitHub Check: 🔬 unit on 3.10
- GitHub Check: 🔬 unit on 3.14
- GitHub Check: 🔬 unit-no-extras on 3.10
- GitHub Check: 🔬 unit-no-extras on 3.13
- GitHub Check: 🔬 integration-postgres on 3.11
- GitHub Check: 🔬 unit on 3.12
- GitHub Check: 🔬 unit on 3.13
- GitHub Check: 🔬 unit-no-extras on 3.11
- GitHub Check: 🔬 unit on 3.11
- GitHub Check: Optimize new code in this PR
🔇 Additional comments (62)
tests/unit/schemas/order/type_order_by.py (1)
15-15: Parameter rename is correctly applied and consistent across the codebase.The change from
order_bytoorderin the @strawchemy.type decorator on line 15 is part of a complete and consistent refactoring. A codebase search confirms that all @strawchemy.type decorator calls now use the neworder=parameter, with no remaining instances of the oldorder_by=parameter at the decorator level. Integration tests and other test files consistently use the updated API.tests/unit/schemas/order/order_config_all_with_field_override.py (1)
1-18: No changes needed. This is a test fixture file, not a test module.The schema file serves as a fixture for the parametrized snapshot test in
tests/unit/mapping/test_schemas.py(line 217). Thetest_query_schemasfunction dynamically imports the Query class from this module and validates the generated GraphQL schema via snapshot assertions. The field-level override (order_by=["name"]) is tested as part of this parametrized test suite.tests/unit/schemas/pagination/paginate_empty.py (1)
1-18: This is an intentional schema fixture used by parametrized snapshot tests.This file is part of the test infrastructure: it defines a GraphQL schema that is dynamically imported and tested by
test_query_schemasintests/unit/mapping/test_schemas.pyvia snapshot assertions. The absence of test functions is correct—all pagination schema definition files follow this pattern. Moving it to a fixtures directory would break the parametrization logic that references the explicit module path.Likely an incorrect or invalid review comment.
tests/unit/schemas/filters/field_filter_auto_generate.py (1)
1-18: Verify purpose: confirm if this is an intentional schema fixture or incomplete test.This file contains only GraphQL schema definitions without test functions or assertions. Based on the file name (
field_filter_auto_generate.py), location (tests/unit/schemas/filters/), and content structure, it appears designed as a reusable schema fixture. However, this cannot be confirmed without examining:
- How this file is imported and used by other tests in the suite
- Naming and organizational conventions established in the codebase
- Whether similar files in the same directory follow the same pattern
If this is intentionally a fixture, ensure it's clearly documented or placed in a dedicated fixtures directory. If it's intended as an executable test, add test functions with assertions.
tests/unit/schemas/order/field_order_by_all.py (1)
1-18: This schema fixture is properly integrated into the test suite.The file is used by the parametrized snapshot test
test_query_schemasintests/unit/mapping/test_schemas.py. The test dynamically imports the Query class and validates the generated GraphQL schema against a snapshot. Schema definition files in this directory intentionally contain no test functions—they are fixtures that define the schemas being tested.Likely an incorrect or invalid review comment.
tests/unit/dto/test_dto.py (1)
271-337: LGTM! Comprehensive test coverage for new DTOConfig methods.The test suite thoroughly covers the new
from_include()factory method andis_field_included()logic across various scenarios:
- None/empty, "all", list, and set inputs
- Custom purpose parameter
- Include/exclude interaction
The test logic is sound and aligns well with the method signatures shown in the relevant code snippets.
tests/unit/schemas/order/field_order_by_specific_fields.py (1)
18-18: Remove or resolve the concern about ordering relationship fields.The
fruitsandvegetablesfields in Container are relationship fields, but the Container model is explicitly documented as a "Test model with multiple list relationships for testing paginate/order configuration." The framework appears to intentionally support ordering by relationship field names. The test file is part of a schema snapshot suite validating GraphQL schema generation, not query execution, and there's no evidence this configuration is invalid.Likely an incorrect or invalid review comment.
tests/unit/schemas/distinct/field_distinct_all.py (2)
1-8: LGTM - Clean test setup.The imports and Strawchemy initialization are correct. The shorthand
Strawchemy("postgresql")is concise for test purposes.
16-18: LGTM - Tests field-level distinct_on configuration.The query correctly demonstrates the new API where
distinct_on="all"is specified at the field level, enabling PostgreSQL's DISTINCT ON clause for all fields of the Color model.tests/unit/schemas/distinct/distinct_config_specific_fields.py (1)
1-8: LGTM - Tests config-level distinct_on with specific fields.The configuration correctly sets
distinct_on=["name"]at the global level, which will be inherited by fields that don't specify their own distinct_on parameter.tests/unit/schemas/distinct/distinct_config_empty.py (1)
11-18: LGTM - Standard type and query definitions.The ColorType and Query definitions follow the established pattern and will inherit the empty
distinct_on=[]from the config.tests/unit/schemas/order/order_config_specific_fields.py (1)
1-8: LGTM - Tests config-level order_by with specific fields.The configuration correctly sets
order_by=["name", "sweetness"]at the global level. Both field names correspond to columns on the Fruit model (name: Mapped[str]at line 59 andsweetness: Mapped[int]at line 62 of tests/unit/models.py).tests/unit/schemas/pagination/pagination_config_with_type_override.py (1)
8-8: The naming convention here is intentional and well-documented.paginationis the global configuration parameter inStrawchemyConfig, whilepaginateis the type decorator parameter for field-level overrides. The codebase explicitly defines this pattern in the type factory's fallback logic (line 276 oftypes.py:paginate=paginate if paginate == "all" else self._mapper.config.pagination) and includes clear documentation in the_add_fields_argumentsdocstring explaining both parameters' purposes. This is standard API design for separating global defaults from type-specific configuration.Likely an incorrect or invalid review comment.
tests/unit/schemas/distinct/distinct_config_with_field_override.py (1)
1-18: LGTM! Clean test structure for distinct_on field override.The test clearly demonstrates field-level override of the global
distinct_on="all"configuration by specifyingdistinct_on=["name"]at the field level. The structure is consistent with other test files in the PR.tests/unit/schemas/pagination/pagination_config_empty_with_type_override.py (1)
1-18: LGTM! Type-level pagination override test is well-structured.The test demonstrates type-level pagination control using
paginate=["fruits"]on the type decorator while the global config haspagination=[]. This validates the type-level override mechanism.src/strawchemy/dto/utils.py (2)
49-50: LGTM! Global include/exclude parameters added consistently.The new
global_includeandglobal_excludeparameters follow the same pattern as the existingincludeandexcludeparameters. The truthiness-based conditional assignment is consistent with the existing code style (see lines 59-62).Note: The truthiness check means empty collections (
[]orset()) won't be assigned. This is consistent with existing behavior but means "explicitly no fields" vs "not specified" cannot be distinguished. This appears intentional based on the codebase patterns.Also applies to: 63-66
22-22: TheExcludeFields→FieldIterabletype rename is complete and consistently applied across the codebase. The type is properly imported on line 22 and used consistently in function parameters on lines 48 and 50. No remaining references to the old type name exist.tests/unit/schemas/pagination/pagination_config_empty.py (1)
1-18: LGTM! Baseline test for empty pagination configuration.This test validates the baseline behavior when pagination is globally disabled using
pagination=[]. It complements the override tests by establishing the default behavior.tests/unit/mapping/test_schemas.py (1)
188-229: LGTM! Comprehensive test coverage expansion.The additions significantly expand test coverage for the refactored pagination, ordering, and distinct_on features:
- Pagination tests (11 new): Cover default limits/offsets, field-specific pagination, config-based variations, and combined scenarios
- Ordering tests (11 new): Cover field-level, type-level, and config-based ordering with various override patterns
- Distinct tests (7 new): Cover config-based and field-level distinct_on with different field specifications
The test parameter naming follows consistent conventions and integrates well with the existing test structure.
src/strawchemy/schema/factories/aggregations.py (1)
126-126: LGTM! Clean threading of the no_cache parameter.The
no_cacheparameter is correctly added with a default value ofFalse, maintaining backward compatibility. The parameter is properly threaded through to the parentfactory()call, consistent with the broader cache control pattern introduced in this PR.Also applies to: 141-141
tests/unit/schemas/distinct/distinct_config_all.py (1)
8-8: Thedistinct_onfeature is not PostgreSQL-specific and does not require dialect validation.The implementation already handles
distinct_onacross all supported databases. PostgreSQL uses nativeDISTINCT ON, while MySQL and SQLite automatically fall back to aRANK()window function. The_use_distinct_rank()method in the transpiler manages this logic transparently, and integration tests confirm the feature works correctly across all database types.tests/unit/schemas/order/order_config_with_field_override.py (1)
8-8: This test validates that field-levelorder_byconfigurations override the global configuration.The field definition includes
order_by=["sweetness"], which overrides the globalorder_by=["name"]configuration. The snapshot test validates this override behavior through GraphQL schema generation comparison. If you need to test global order_by behavior when no field-level override is present, refer toorder_config_all.pyororder_config_specific_fields.pyinstead.Likely an incorrect or invalid review comment.
tests/unit/schemas/order/order_config_specific_fields_with_type_override.py (1)
11-11: Verify that selective ordering is properly tested for thecolorsrelationship.The
order=["fruits", "vegetables"]decorator correctly enables ordering only for those two relationships. Based on code analysis, the_add_fields_argumentsmethod filters which relation fields receiveorder_byarguments by checkingorder_config.is_field_included(field.model_field_name). Thecolorsrelationship, which exists on the Container model but is not in the order list, will not receive ordering arguments in the generated GraphQL schema. Ensure the test assertions or generated schema snapshots explicitly validate this behavior.tests/unit/schemas/order/type_order_by_specific_fields.py (3)
1-8: LGTM!The imports and Strawchemy initialization are correct and align with the test module's purpose.
11-12: LGTM!The type definition correctly uses the new
orderparameter with a field list, demonstrating the refactored API for specifying orderable fields.
15-17: LGTM!The Query type definition follows the standard pattern for exposing Strawchemy-backed fields in Strawberry GraphQL schemas.
tests/unit/schemas/order/order_config_empty.py (3)
1-8: LGTM!The test correctly validates that an empty
order_bylist is accepted inStrawchemyConfig, providing useful edge-case coverage.
11-13: LGTM!The type definition correctly relies on the config-level
order_bysetting without a type-level override.
16-18: LGTM!The Query definition follows the established pattern for Strawberry GraphQL schemas.
src/strawchemy/validation/pydantic.py (3)
27-27: LGTM!The import change from
ExcludeFieldstoFieldIterablealigns with the broader type system refactor to unify field-specification types across the DTO layer.
87-87: LGTM!The parameter type change to
FieldIterable | Noneis consistent with the updated import and maintains API compatibility.
112-127: LGTM!The
no_cacheparameter addition follows the broader factory interface pattern and is correctly forwarded to the superclass. The default value ofFalsemaintains backward compatibility.tests/unit/schemas/pagination/pagination_config_specific_fields.py (3)
1-8: LGTM!The test setup correctly demonstrates the new per-field pagination configuration API, enabling pagination only for the "fruits" and "vegetables" relationships.
11-13: LGTM!The type definition correctly relies on the config-level pagination setting, demonstrating how global pagination configuration is applied without type-level overrides.
16-18: LGTM!The Query definition follows the standard Strawberry GraphQL schema pattern.
src/strawchemy/schema/factories/enum.py (4)
11-14: LGTM!The import changes support the new
make_inputmethod and align with the DTO type system refactor to useFieldIterableinstead ofExcludeFields.
145-145: LGTM!The parameter type change to
FieldIterable | Nonemaintains consistency with the updated type system across the codebase.
164-193: LGTM with minor observation!The new
make_inputmethod provides a clean programmatic API for creating enum input types with a WRITE-purpose DTOConfig. The implementation correctly constructs the config and forwards all parameters to the factory method.Note: Static analysis flags the boolean parameters (
partial,no_cache) as positional arguments (FBT001/FBT002). These warnings can be safely ignored as they're optional configuration parameters following the established pattern across the factory API, and changing them to keyword-only would break consistency with the existing codebase.
199-199: LGTM!The parameter type change to
FieldIterable | Noneis consistent with the decorator method and the broader type system refactor.src/strawchemy/schema/mutation/field_builder.py (2)
18-19: LGTM! Factory dependencies properly added.The new factory imports and dataclass fields correctly extend
MutationFieldBuilderwith the necessary factory dependencies (order_by_factory,filter_factory,distinct_on_factory) to support the enhanced pagination and ordering API.Also applies to: 40-42
95-114: Remove this comment - fallback logic is correctly implemented.The
repository_typeparameter is handled with a proper fallback in the parent classStrawchemyField.__init__(line 132 infield.py):self._repository_type = config.repository_type if repository_type is None else repository_type. This ensures that whenNoneis passed, the config's default repository type is used, making the mutation field classes safe and properly defensive against None values.Likely an incorrect or invalid review comment.
src/strawchemy/utils/registry.py (1)
105-108: LGTM! Registry type info extended for pagination and ordering.The new fields (
pagination,order,distinct_on,paginate) correctly extendRegistryTypeInfoto carry pagination and ordering configuration through the type registration system. The use offrozensetas default factory ensures immutability.src/strawchemy/dto/base.py (3)
394-414: LGTM! Global include/exclude logic is correctly implemented.The updated
should_exclude_fieldmethod properly handles the new global inclusion/exclusion semantics:
- Root nodes use explicit
include/exclude- Non-root nodes use
global_include/global_exclude- The
include="all"andglobal_include="all"cases are handled correctly
638-638: LGTM! Cache bypass withno_cacheparameter.The
no_cacheparameter is correctly implemented to bypass the DTO cache when needed. Regarding the static analysis hint (FBT001/FBT002) about boolean positional arguments: this is acceptable here sinceno_cachefollows other positional parameters and maintains consistency with the existing API pattern.Also applies to: 653-653
698-698: LGTM! Type annotation updated toFieldIterable.The
excludeparameter type is correctly updated fromExcludeFieldstoFieldIterable, aligning with the PR's type refactoring.src/strawchemy/schema/factories/inputs.py (1)
55-55: LGTM! Type updates and aggregation field configuration.The
FieldIterabletype annotation and the addition ofpartial=Trueincopy_withfor aggregation fields are correctly implemented.Also applies to: 146-148
src/strawchemy/schema/factories/types.py (3)
107-205: Well-documented refactoring of_add_fields_arguments.The method is comprehensively refactored to handle:
- Ordering arguments via
orderparameter with properDTOConfig.from_includeconversion- Pagination arguments via
paginateparameter- JSON path filtering for JSON/dict fields
The try/except pattern around
EmptyDTOError(lines 170-173) is good defensive coding to handle models with no orderable fields.
239-302: LGTM! Factory method properly extended with new parameters.The factory method correctly:
- Adds
default_pagination,order, andpaginateparameters- Falls back to
self._mapper.configvalues when appropriate (lines 276-278)- Only calls
_add_fields_argumentsfor object types (line 281)- Propagates new parameters to
_register_type
607-621: The cache key implementation is correct and does not have collision risks. Thefactory_kwargsparameter was never used in the parent class's cache key computation (only passed through as a parameter), andmodeis properly included in the tuple returned at line 620, ensuring distinct cache keys for differentmodevalues.src/strawchemy/dto/types.py (5)
35-36: LGTM! Type aliases for field configuration.The
FieldIterableandIncludeFieldstype aliases provide clear, expressive types for the new configuration API.
165-170: LGTM! Global include/exclude fields with proper validation.The new
global_includeandglobal_excludefields extend DTOConfig for tree-wide field control. The__post_init__validation correctly enforces:
- Mutual exclusivity between explicit include sets and exclude sets
- Auto-setting
global_include="all"whenglobal_excludeis specifiedAlso applies to: 184-197
199-220: LGTM! Factory methodfrom_includeis well-documented.The
from_includeclassmethod provides a clean factory interface for creating DTOConfig instances from include specifications. The docstring clearly explains the parameter semantics.
312-332: LGTM!is_field_includedmethod correctly implements inclusion logic.The method properly evaluates field inclusion by checking:
include="all"→ field is included unless inexcludeglobal_include="all"→ field is included unless inglobal_exclude- Otherwise → field must be in combined include sets and not in combined exclude sets
335-359: LGTM! Type-safe utility functions.
cast_include_fields: Properly overloaded for precise return types with correct match-case implementationis_fields_iterable: Type guard function that correctly validates field iterables while rejecting plain stringsThe static analysis hint (ANN401) about
Anyinis_fields_iterableis acceptable since this is a type guard function that needs to accept arbitrary input to validate.src/strawchemy/mapper.py (2)
139-145:MutationFieldBuilderinitialization via explicit keyword args is a net win.
Much harder to break during refactors than positional args.
173-228: No action needed. The only directStrawchemyFieldinstantiation in the codebase (at mapper.py:296) already passes all three required factories:order_by_factory,filter_factory, anddistinct_on_factory. No other call sites require updating.Likely an incorrect or invalid review comment.
src/strawchemy/schema/field.py (2)
81-135: Constructor refactor to factory-driven DTO creation is coherent (and defaults are centralized).
Defaultingid_field_name,execution_options, andrepository_typefrom config makes behavior easier to reason about.
238-246: Pagination defaulting viaconfig.pagination == "all"is OK, but confirm intended precedence ofFalsevsNone.
Right now_pagination=Falsedisables pagination even if config is"all", while_pagination=Noneinherits config. That’s a good model—just ensure it’s the documented one.src/strawchemy/schema/factories/base.py (1)
192-216: Threadingglobal_include/global_excludeinto config is the right direction.
Nice alignment withDTOConfig.__post_init__behavior and global semantics.tests/unit/schemas/order/order_config_empty_with_empty_type_override.py (3)
1-7: LGTM: Imports are appropriate.The imports are correct for this test schema definition. The future annotations import enables forward references for type hints.
16-18: LGTM: Query type follows standard pattern.The Query type correctly uses the Strawberry/Strawchemy integration pattern with
strawchemy.field()for GraphQL field resolution.
8-8: This follows the established pattern across the test suite and requires no changes.Module-level
Strawchemyinstances are the standard design pattern used throughout the codebase in all test schema definition files (100+ instances found acrosstests/unit/schemas/andtests/integration/). Each test module creates its own isolated instance with its specific configuration, which is then imported by test functions. This approach properly isolates schema definitions per test and is the intended design.Likely an incorrect or invalid review comment.
|
|
||
|
|
||
| @strawchemy.type(Ticket, include="all", filter_input=TicketFilter, order_by=TicketOrder, override=True) | ||
| @strawchemy.type(Ticket, include="all", filter_input=TicketFilter, order=TicketOrder, override=True) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for order_by usage in strawchemy.type() and strawchemy.field() calls
echo "=== Type decorator usage with order_by ==="
rg -nP '@strawchemy\.type\([^)]*order_by\s*=' --type=py
echo -e "\n=== Field usage with order_by ==="
rg -nP 'strawchemy\.field\([^)]*order_by\s*=' --type=py
echo -e "\n=== Type decorator usage with order= (new pattern) ==="
rg -nP '@strawchemy\.type\([^)]*order\s*=' --type=py | head -20
echo -e "\n=== Field usage with order= (new pattern) ==="
rg -nP 'strawchemy\.field\([^)]*order\s*=' --type=py | head -20Repository: gazorby/strawchemy
Length of output: 3173
Update field-level API to use order= parameter for consistency.
The type decorator has been migrated to use order=, but strawchemy.field() calls still use order_by=. This creates an API inconsistency that needs to be resolved:
- Type-level: Uses
order=(e.g., line 23 of examples/testapp/testapp/types.py) - Field-level: Still uses
order_by=(found in 7 test files including tests/unit/schemas/order/field_order_by_specific_fields.py:18)
Update all field-level calls to use order= to match the type-level API.
🤖 Prompt for AI Agents
In @examples/testapp/testapp/types.py at line 23, The project mixes two ordering
parameters: the type decorator uses order= (e.g., @strawchemy.type(Ticket,
include="all", filter_input=TicketFilter, order=TicketOrder, override=True))
while field-level calls still use order_by=; update all strawchemy.field(...)
usages to replace order_by= with order= so the field-level API matches the
type-level API (search for strawchemy.field in tests and examples, e.g.,
tests/unit/schemas/order/field_order_by_specific_fields.py and
examples/testapp/testapp/types.py references to
Ticket/TicketFilter/TicketOrder).
| include: IncludeFields = "all" | ||
| """Globally included fields.""" | ||
| exclude: FieldIterable | None = None | ||
| """Globally included fields.""" | ||
| pagination: IncludeFields | None = None | ||
| """Enable/disable pagination on list resolvers.""" | ||
| order_by: IncludeFields | None = None | ||
| """Enable/disable order by on list resolvers.""" | ||
| distinct_on: IncludeFields | None = None | ||
| """Enable/disable order by onelist resolvers.""" | ||
| pagination_default_limit: int = 100 | ||
| """Default pagination limit when `pagination=True`.""" | ||
| pagination_default_offset: int = 0 | ||
| """Default pagination offset when `pagination=True`.""" |
There was a problem hiding this comment.
Fix documentation errors in field docstrings.
There are two issues in the docstrings:
- Line 54: The
excludefield docstring says "Globally included fields" but should describe excluded fields - Line 60: Typo "onelist" should be "on list"
📝 Proposed fix
include: IncludeFields = "all"
"""Globally included fields."""
exclude: FieldIterable | None = None
- """Globally included fields."""
+ """Globally excluded fields."""
pagination: IncludeFields | None = None
"""Enable/disable pagination on list resolvers."""
order_by: IncludeFields | None = None
"""Enable/disable order by on list resolvers."""
distinct_on: IncludeFields | None = None
- """Enable/disable order by onelist resolvers."""
+ """Enable/disable distinct on for list resolvers."""
pagination_default_limit: int = 100
"""Default pagination limit when `pagination=True`."""
pagination_default_offset: int = 0
"""Default pagination offset when `pagination=True`."""🤖 Prompt for AI Agents
In @src/strawchemy/config/base.py around lines 51 - 64, Update the field
docstrings to correct their descriptions: change the docstring for the exclude
field (symbol: exclude: FieldIterable | None) from "Globally included fields."
to something like "Globally excluded fields." and fix the typo in the
distinct_on field docstring (symbol: distinct_on: IncludeFields | None) from
"Enable/disable order by onelist resolvers." to "Enable/disable order by on list
resolvers."
| __slots__ = ("_key",) | ||
|
|
||
| separator: ClassVar[str] = ":" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the _Key class definition around lines 132-134
sed -n '120,160p' src/strawchemy/dto/strawberry.pyRepository: gazorby/strawchemy
Length of output: 1545
🏁 Script executed:
# Find all subclasses of _Key in the file
rg -n 'class\s+\w+.*\(_Key\)' src/strawchemy/dto/strawberry.py -A 5Repository: gazorby/strawchemy
Length of output: 44
🏁 Script executed:
# Check the DTOKey class mentioned at line 179
sed -n '175,210p' src/strawchemy/dto/strawberry.pyRepository: gazorby/strawchemy
Length of output: 1081
🏁 Script executed:
# Search for all classes that inherit from _Key
rg -n '_Key' src/strawchemy/dto/strawberry.py | grep -E '(class|:)'Repository: gazorby/strawchemy
Length of output: 124
🏁 Script executed:
# Check for dynamic attribute assignments or setattr patterns
rg -n 'setattr|__dict__|self\.\w+\s*=' src/strawchemy/dto/strawberry.py | head -20Repository: gazorby/strawchemy
Length of output: 340
🏁 Script executed:
# Look at git history to verify if separator was changed from instance to ClassVar
git log -p --all -S 'separator' -- src/strawchemy/dto/strawberry.py | head -100Repository: gazorby/strawchemy
Length of output: 44
🏁 Script executed:
# Check if DTOKey or its instances need to store additional attributes
rg -n 'DTOKey' src/strawchemy/dto/strawberry.py -B 2 -A 10 | head -50Repository: gazorby/strawchemy
Length of output: 2047
🏁 Script executed:
# Check if there are any tests that might reveal the actual impact
find . -name '*test*.py' -type f | head -10Repository: gazorby/strawchemy
Length of output: 474
🏁 Script executed:
# Look for all usages of DTOKey to understand its instantiation patterns
rg -n 'DTOKey' --type py | head -20Repository: gazorby/strawchemy
Length of output: 1932
🏁 Script executed:
# Verify that DTOKey doesn't add any additional attributes by checking all its methods
sed -n '179,200p' src/strawchemy/dto/strawberry.pyRepository: gazorby/strawchemy
Length of output: 738
🏁 Script executed:
# Check if DTOKey instances are created in large quantities (memory-critical context)
rg -n 'DTOKey\(' src/strawchemy -c --type py | sort -rnRepository: gazorby/strawchemy
Length of output: 140
🏁 Script executed:
# Look at the parent _Key's full implementation to understand inheritance model
sed -n '114,176p' src/strawchemy/dto/strawberry.pyRepository: gazorby/strawchemy
Length of output: 2205
🏁 Script executed:
# Verify if there are any other subclasses of _Key besides DTOKey
rg -n 'class\s+\w+.*\(_Key' src/ --type pyRepository: gazorby/strawchemy
Length of output: 129
🏁 Script executed:
# Check if separator usage would be different if it were an instance variable vs ClassVar
rg -n 'separator' src/strawchemy/dto/strawberry.py -B 1 -A 1Repository: gazorby/strawchemy
Length of output: 479
🏁 Script executed:
# Verify Python behavior: check if empty __slots__ in subclass prevents __dict__
python3 << 'EOF'
# Test: Does subclass without __slots__ get a __dict__?
class Parent:
__slots__ = ("x",)
class Child(Parent):
pass
c = Child()
print("Child has __dict__:", hasattr(c, '__dict__'))
print("Parent __slots__:", Parent.__slots__)
print("Child __slots__:", getattr(Child, '__slots__', 'Not defined'))
# With __slots__ = () in child
class ChildWithSlots(Parent):
__slots__ = ()
c2 = ChildWithSlots()
print("\nChildWithSlots has __dict__:", hasattr(c2, '__dict__'))
print("ChildWithSlots __slots__:", ChildWithSlots.__slots__)
EOFRepository: gazorby/strawchemy
Length of output: 148
🏁 Script executed:
# Simpler test for Python __slots__ behavior
python3 << 'EOF'
class Parent:
__slots__ = ("x",)
class Child(Parent):
pass
try:
c = Child()
c.y = 5
print("Child WITHOUT __slots__ definition can add attributes: True")
except AttributeError:
print("Child WITHOUT __slots__ definition blocked attribute addition")
EOFRepository: gazorby/strawchemy
Length of output: 121
🏁 Script executed:
# Check how many DTOKey instances are typically created in real usage
rg -n 'DTOKey\(' src/strawchemy/schema/factories/ -B 2 -A 2 | head -30Repository: gazorby/strawchemy
Length of output: 2055
🏁 Script executed:
# Verify if DTOKey instances are stored in long-lived data structures
rg -n '__strawchemy_field_map__' src/strawchemy -A 1 | head -20Repository: gazorby/strawchemy
Length of output: 1639
Fix DTOKey to define __slots__ to complete the memory optimization.
The addition of __slots__ in _Key prevents per-instance __dict__ creation, and using separator as a ClassVar properly shares it across instances. However, DTOKey (line 179) does not define __slots__, which causes each instance to gain a __dict__ despite inheriting from _Key, defeating the optimization. Since DTOKey instances are stored as keys in __strawchemy_field_map__ dictionaries and persist for the lifetime of the schema, adding __slots__ = () to DTOKey will complete the optimization.
🤖 Prompt for AI Agents
In @src/strawchemy/dto/strawberry.py around lines 132 - 134, DTOKey is missing a
__slots__ declaration so instances still get per-instance __dict__ despite
inheriting from _Key; add a class-level __slots__ = () to the DTOKey class (the
DTOKey class definition referenced in the diff) to prevent per-instance
dictionaries and complete the memory optimization already begun by _Key and the
separator ClassVar.
| _types_namespace: ClassVar[dict[str, Any]] = TYPING_NS | vars(mutation_types) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the target file
cat -n src/strawchemy/mapper.py | head -100Repository: gazorby/strawchemy
Length of output: 5007
🏁 Script executed:
# Check Python version requirements
fd -e "toml|yaml|txt|cfg|ini" | xargs grep -l "python" | head -10Repository: gazorby/strawchemy
Length of output: 44
🏁 Script executed:
# Find Python version specification
cat -n pyproject.toml | head -50Repository: gazorby/strawchemy
Length of output: 2167
🏁 Script executed:
# See lines 149-157 in mapper.py
sed -n '140,165p' src/strawchemy/mapper.pyRepository: gazorby/strawchemy
Length of output: 1051
🏁 Script executed:
# Check what mutation_types module contains
cat -n src/strawchemy/schema/mutation/types.pyRepository: gazorby/strawchemy
Length of output: 9132
🏁 Script executed:
# Check what TYPING_NS contains
cat -n src/strawchemy/dto/base.py | head -100Repository: gazorby/strawchemy
Length of output: 3127
🏁 Script executed:
# Check what vars(mutation_types) actually contains
python3 << 'EOF'
import sys
# Show what module vars includes
test_dict = vars(sys)
print("Sample module vars() keys:")
print([k for k in list(test_dict.keys())[:15]])
print("\nContains dunders:", any(k.startswith("__") for k in test_dict.keys()))
print("Dunder examples:", [k for k in test_dict.keys() if k.startswith("__")][:5])
EOFRepository: gazorby/strawchemy
Length of output: 419
🏁 Script executed:
# Verify the exact context of line 149 in mapper.py
sed -n '145,160p' src/strawchemy/mapper.pyRepository: gazorby/strawchemy
Length of output: 600
🏁 Script executed:
# Check if there are other uses of vars() in the codebase that might need fixing
rg 'vars\(' --type py -A 1 -B 1Repository: gazorby/strawchemy
Length of output: 982
🏁 Script executed:
# Check if Strawberry actually cares about dunders in namespace during type resolution
# Look for how _annotation_namespace is used in the codebase
rg '_annotation_namespace|annotation_namespace' --type py -B 2 -A 2 | head -60Repository: gazorby/strawchemy
Length of output: 1136
🏁 Script executed:
# See if filtering dunders in one place but not TYPING_NS creates inconsistency
# Check what TYPING_NS is actually used for
rg 'TYPING_NS' --type py -B 1 -A 1Repository: gazorby/strawchemy
Length of output: 1609
🏁 Script executed:
# Check if Strawberry's from_annotation has documented issues with dunders
# Look for any tests or issues that might reveal the actual impact
rg 'from_annotation' --type py -B 2 -A 3 | head -40Repository: gazorby/strawchemy
Length of output: 1404
🏁 Script executed:
# Examine the specific pattern: does filtering dunders only from mutation_types
# but not from TYPING_NS create an inconsistency?
grep -n 'TYPING_NS' src/strawchemy/mapper.pyRepository: gazorby/strawchemy
Length of output: 191
🏁 Script executed:
# Check mutation_types module to see what actual types/values it exports
cat -n src/strawchemy/schema/mutation/types.py | head -50Repository: gazorby/strawchemy
Length of output: 1814
Filter dunders from the type annotation namespace, but address both sources consistently.
Line 86 uses vars(mutation_types) which includes module dunders (__name__, __doc__, etc.), polluting the type namespace unnecessarily. However, the fix must also filter TYPING_NS since it's defined as vars(typing) | vars(types) (line 73 in dto/base.py), which contains the same dunder pollution.
Replace both usages:
- Line 86 in
src/strawchemy/mapper.py: Filter dunders from bothTYPING_NSandvars(mutation_types) - Line 73 in
src/strawchemy/dto/base.py: Filter dunders fromvars(typing)andvars(types)
The note about dict union (|) requiring Python 3.9+ is not applicable—the project requires Python 3.10+. The reference to "Also applies to: 149-157" is incorrect; those lines contain the _annotation_namespace method which uses dict union but not vars() directly.
🤖 Prompt for AI Agents
In @src/strawchemy/mapper.py around lines 86 - 87, Tidy the type annotation
namespaces by removing dunder entries before performing the dict unions: when
building TYPING_NS in dto/base.py (symbol TYPING_NS) filter vars(typing) and
vars(types) to exclude keys that start and end with "__" and use those cleaned
dicts for the union; similarly in mapper.py (symbol _types_namespace) filter
vars(mutation_types) to remove dunder keys and also ensure you union against the
already-cleaned TYPING_NS so both sources are consistently dunder-free.
| return self.make_input( | ||
| model=model, | ||
| mode=mode, | ||
| include=include, | ||
| exclude=exclude, | ||
| partial=partial, | ||
| type_map=type_map, | ||
| alias_generator=alias_generator, | ||
| aliases=aliases, | ||
| scope=scope, | ||
| tags={mode}, | ||
| ) | ||
| dto = self.factory( | ||
| model=model, | ||
| dto_config=dto_config, | ||
| base=class_, | ||
| alias_generator=alias_generator, | ||
| name=name, | ||
| description=description, | ||
| directives=directives, | ||
| override=override, | ||
| user_defined=True, | ||
| mode=mode, | ||
| purpose=purpose, | ||
| scope=scope, | ||
| base=class_, | ||
| **kwargs, | ||
| ) | ||
| dto.__strawchemy_purpose__ = mode | ||
| return dto | ||
|
|
||
| return wrapper | ||
|
|
||
| def make_input( | ||
| self, | ||
| model: type[T], | ||
| *, | ||
| mode: GraphQLPurpose, | ||
| include: IncludeFields | None = None, | ||
| exclude: FieldIterable | None = None, | ||
| partial: bool | None = None, | ||
| type_map: Mapping[Any, Any] | None = None, | ||
| aliases: Mapping[str, str] | None = None, | ||
| alias_generator: Callable[[str], str] | None = None, | ||
| name: str | None = None, | ||
| description: str | None = None, | ||
| directives: Sequence[object] | None = (), | ||
| override: bool = False, | ||
| purpose: Purpose = Purpose.WRITE, | ||
| scope: DTOScope | None = None, | ||
| base: type[Any] | None = None, | ||
| user_defined: bool = False, | ||
| no_cache: bool = False, | ||
| **kwargs: Any, | ||
| ) -> type[GraphQLDTOT]: | ||
| dto_config = self._config( | ||
| purpose, | ||
| include=include, | ||
| exclude=exclude, | ||
| partial=partial, | ||
| type_map=type_map, | ||
| alias_generator=alias_generator, | ||
| aliases=aliases, | ||
| scope=scope, | ||
| tags={mode}, | ||
| ) | ||
| if no_cache: | ||
| dto_name = name or self.root_dto_name(model, dto_config) | ||
| name = self._mapper.registry.uniquify_name(self.graphql_type(dto_config), dto_name) | ||
| dto = self.factory( | ||
| model=model, | ||
| dto_config=dto_config, | ||
| base=base, | ||
| name=name, | ||
| description=description, | ||
| directives=directives, | ||
| override=override, | ||
| user_defined=user_defined, | ||
| mode=mode, | ||
| no_cache=no_cache, | ||
| **kwargs, | ||
| ) | ||
| dto.__strawchemy_purpose__ = mode | ||
| return dto | ||
|
|
There was a problem hiding this comment.
make_input() is a good consolidation, but address Ruff ANN401 on **kwargs: Any.
If Ruff is enforced, this will fail CI (static analysis hint). Prefer object (or a typed Unpack[...] if you have a stable kwargs shape).
Proposed fix
@@
def make_input(
@@
- **kwargs: Any,
+ **kwargs: object,
) -> type[GraphQLDTOT]:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return self.make_input( | |
| model=model, | |
| mode=mode, | |
| include=include, | |
| exclude=exclude, | |
| partial=partial, | |
| type_map=type_map, | |
| alias_generator=alias_generator, | |
| aliases=aliases, | |
| scope=scope, | |
| tags={mode}, | |
| ) | |
| dto = self.factory( | |
| model=model, | |
| dto_config=dto_config, | |
| base=class_, | |
| alias_generator=alias_generator, | |
| name=name, | |
| description=description, | |
| directives=directives, | |
| override=override, | |
| user_defined=True, | |
| mode=mode, | |
| purpose=purpose, | |
| scope=scope, | |
| base=class_, | |
| **kwargs, | |
| ) | |
| dto.__strawchemy_purpose__ = mode | |
| return dto | |
| return wrapper | |
| def make_input( | |
| self, | |
| model: type[T], | |
| *, | |
| mode: GraphQLPurpose, | |
| include: IncludeFields | None = None, | |
| exclude: FieldIterable | None = None, | |
| partial: bool | None = None, | |
| type_map: Mapping[Any, Any] | None = None, | |
| aliases: Mapping[str, str] | None = None, | |
| alias_generator: Callable[[str], str] | None = None, | |
| name: str | None = None, | |
| description: str | None = None, | |
| directives: Sequence[object] | None = (), | |
| override: bool = False, | |
| purpose: Purpose = Purpose.WRITE, | |
| scope: DTOScope | None = None, | |
| base: type[Any] | None = None, | |
| user_defined: bool = False, | |
| no_cache: bool = False, | |
| **kwargs: Any, | |
| ) -> type[GraphQLDTOT]: | |
| dto_config = self._config( | |
| purpose, | |
| include=include, | |
| exclude=exclude, | |
| partial=partial, | |
| type_map=type_map, | |
| alias_generator=alias_generator, | |
| aliases=aliases, | |
| scope=scope, | |
| tags={mode}, | |
| ) | |
| if no_cache: | |
| dto_name = name or self.root_dto_name(model, dto_config) | |
| name = self._mapper.registry.uniquify_name(self.graphql_type(dto_config), dto_name) | |
| dto = self.factory( | |
| model=model, | |
| dto_config=dto_config, | |
| base=base, | |
| name=name, | |
| description=description, | |
| directives=directives, | |
| override=override, | |
| user_defined=user_defined, | |
| mode=mode, | |
| no_cache=no_cache, | |
| **kwargs, | |
| ) | |
| dto.__strawchemy_purpose__ = mode | |
| return dto | |
| return self.make_input( | |
| model=model, | |
| mode=mode, | |
| include=include, | |
| exclude=exclude, | |
| partial=partial, | |
| type_map=type_map, | |
| aliases=aliases, | |
| alias_generator=alias_generator, | |
| name=name, | |
| description=description, | |
| directives=directives, | |
| override=override, | |
| purpose=purpose, | |
| scope=scope, | |
| base=class_, | |
| **kwargs, | |
| ) | |
| return wrapper | |
| def make_input( | |
| self, | |
| model: type[T], | |
| *, | |
| mode: GraphQLPurpose, | |
| include: IncludeFields | None = None, | |
| exclude: FieldIterable | None = None, | |
| partial: bool | None = None, | |
| type_map: Mapping[Any, Any] | None = None, | |
| aliases: Mapping[str, str] | None = None, | |
| alias_generator: Callable[[str], str] | None = None, | |
| name: str | None = None, | |
| description: str | None = None, | |
| directives: Sequence[object] | None = (), | |
| override: bool = False, | |
| purpose: Purpose = Purpose.WRITE, | |
| scope: DTOScope | None = None, | |
| base: type[Any] | None = None, | |
| user_defined: bool = False, | |
| no_cache: bool = False, | |
| **kwargs: object, | |
| ) -> type[GraphQLDTOT]: | |
| dto_config = self._config( | |
| purpose, | |
| include=include, | |
| exclude=exclude, | |
| partial=partial, | |
| type_map=type_map, | |
| alias_generator=alias_generator, | |
| aliases=aliases, | |
| scope=scope, | |
| tags={mode}, | |
| ) | |
| if no_cache: | |
| dto_name = name or self.root_dto_name(model, dto_config) | |
| name = self._mapper.registry.uniquify_name(self.graphql_type(dto_config), dto_name) | |
| dto = self.factory( | |
| model=model, | |
| dto_config=dto_config, | |
| base=base, | |
| name=name, | |
| description=description, | |
| directives=directives, | |
| override=override, | |
| user_defined=user_defined, | |
| mode=mode, | |
| no_cache=no_cache, | |
| **kwargs, | |
| ) | |
| dto.__strawchemy_purpose__ = mode | |
| return dto |
🧰 Tools
🪛 Ruff (0.14.10)
342-342: Dynamically typed expressions (typing.Any) are disallowed in **kwargs
(ANN401)
🤖 Prompt for AI Agents
In @src/strawchemy/schema/factories/base.py around lines 301 - 373, The
make_input signature currently uses an untyped **kwargs: Any which triggers Ruff
ANN401; change the var-keyword param to **kwargs: object (or to a concrete
Unpack[...] type if you have a stable shape) in the make_input definition and
update any related type annotations that depend on it, and keep the forwarding
call to self.factory(model=..., **kwargs) as-is; specifically edit the
make_input function's parameter list (the **kwargs: Any token) and ensure usages
like the call to self.factory remain compatible with the new **kwargs: object
typing.
| @strawchemy.type(Container, include="all", order=[]) | ||
| class ContainerType: | ||
| pass |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add docstring to document test scenario.
The class lacks documentation explaining the test scenario: validating behavior when include="all" is combined with an empty type-level ordering override (order=[]). A docstring would clarify the intent for future maintainers.
📝 Suggested documentation
@strawchemy.type(Container, include="all", order=[])
class ContainerType:
- pass
+ """Test schema: include all fields with empty type-level order override."""
+ pass📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @strawchemy.type(Container, include="all", order=[]) | |
| class ContainerType: | |
| pass | |
| @strawchemy.type(Container, include="all", order=[]) | |
| class ContainerType: | |
| """Test schema: include all fields with empty type-level order override.""" | |
| pass |
🤖 Prompt for AI Agents
In @tests/unit/schemas/order/order_config_empty_with_empty_type_override.py
around lines 11 - 13, Add a docstring to the ContainerType class describing the
test scenario: that it verifies behavior when a schema uses include="all" on
Container while a type-level ordering override is explicitly empty (order=[]),
clarifying expected outcome and any edge conditions; place the docstring
immediately under the class declaration for ContainerType and mention the
relationship to Container, include="all", and the empty order override so future
maintainers understand the intent.
| from __future__ import annotations | ||
|
|
||
| import strawberry | ||
|
|
||
| from strawchemy import Strawchemy | ||
| from tests.unit.models import Container | ||
|
|
||
| strawchemy = Strawchemy("postgresql") | ||
|
|
||
|
|
||
| @strawchemy.type(Container, include="all", order=["fruits", "vegetables"]) | ||
| class ContainerType: | ||
| pass | ||
|
|
||
|
|
||
| @strawberry.type | ||
| class Query: | ||
| container: list[ContainerType] = strawchemy.field() |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add module docstring to explain selective field ordering.
This schema demonstrates selective ordering control at the type level, where only specific relationship fields (fruits and vegetables) support ordering, while others (e.g., colors) do not. A docstring would clarify this behavior.
📝 Suggested docstring
+"""Test schema with selective type-level ordering.
+
+Demonstrates order=["fruits", "vegetables"] to enable ordering only for
+specific relationship fields, while excluding others (e.g., colors).
+"""
from __future__ import annotations
import strawberry📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from __future__ import annotations | |
| import strawberry | |
| from strawchemy import Strawchemy | |
| from tests.unit.models import Container | |
| strawchemy = Strawchemy("postgresql") | |
| @strawchemy.type(Container, include="all", order=["fruits", "vegetables"]) | |
| class ContainerType: | |
| pass | |
| @strawberry.type | |
| class Query: | |
| container: list[ContainerType] = strawchemy.field() | |
| """Test schema with selective type-level ordering. | |
| Demonstrates order=["fruits", "vegetables"] to enable ordering only for | |
| specific relationship fields, while excluding others (e.g., colors). | |
| """ | |
| from __future__ import annotations | |
| import strawberry | |
| from strawchemy import Strawchemy | |
| from tests.unit.models import Container | |
| strawchemy = Strawchemy("postgresql") | |
| @strawchemy.type(Container, include="all", order=["fruits", "vegetables"]) | |
| class ContainerType: | |
| pass | |
| @strawberry.type | |
| class Query: | |
| container: list[ContainerType] = strawchemy.field() |
🤖 Prompt for AI Agents
In @tests/unit/schemas/order/order_config_specific_fields_with_type_override.py
around lines 1 - 18, Add a top-of-file module docstring explaining that this
schema demonstrates selective field ordering at the type level: ContainerType
(created via Strawchemy("postgresql") and annotated with @strawchemy.type(...,
include="all", order=["fruits", "vegetables"])) only exposes ordering for the
specified relationship fields ("fruits" and "vegetables") while other
relationships like "colors" do not support ordering; ensure the docstring
briefly states the purpose and how to interpret the Query.container field
populated via strawchemy.field().
| @strawchemy.type(Fruit, include="all") | ||
| class FruitType: | ||
| pass | ||
|
|
||
|
|
||
| @strawberry.type | ||
| class Query: | ||
| fruits: list[FruitType] = strawchemy.field() |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add documentation to clarify config inheritance.
The test demonstrates config-level order_by inheritance but lacks documentation explaining this behavior.
📝 Suggested documentation
+"""Test schema demonstrating config-level order_by inheritance.
+
+The order_by=['name', 'sweetness'] setting at config level is inherited by the fruits field,
+enabling ordering on these specific columns by default.
+"""
+
from __future__ import annotationsCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @tests/unit/schemas/order/order_config_specific_fields.py around lines 11 -
18, Add a short explanatory comment or docstring above the test types to
document how config-level order_by inheritance works: mention that the
@strawchemy.type(Fruit, include="all") applied on FruitType inherits ordering
config from Fruit, and that Query.fruits uses FruitType so the test validates
that inherited order_by rules apply; place the note near the FruitType or Query
declaration referencing the symbols Fruit, FruitType, and Query so future
readers understand the intended behavior.
| from __future__ import annotations | ||
|
|
||
| import strawberry | ||
|
|
||
| from strawchemy import Strawchemy, StrawchemyConfig | ||
| from tests.unit.models import Fruit | ||
|
|
||
| strawchemy = Strawchemy(StrawchemyConfig("postgresql", order_by=["name"])) | ||
|
|
||
|
|
||
| @strawchemy.type(Fruit, include="all") | ||
| class FruitType: | ||
| pass | ||
|
|
||
|
|
||
| @strawberry.type | ||
| class Query: | ||
| fruits: list[FruitType] = strawchemy.field(order_by=["sweetness"]) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add module docstring to clarify test intent.
This test schema demonstrates field-level order_by override of global configuration, but lacks a docstring explaining its purpose. Consider adding a module docstring to improve maintainability.
📝 Suggested docstring
+"""Test schema demonstrating field-level order_by override of global ordering configuration.
+
+Global config sets order_by=["name"], but the fruits field overrides with order_by=["sweetness"].
+"""
from __future__ import annotations
import strawberry🤖 Prompt for AI Agents
In @tests/unit/schemas/order/order_config_with_field_override.py around lines 1
- 18, Add a short module-level docstring at the top of this test describing its
intent: explain that the global StrawchemyConfig is set with order_by=["name"]
but the Query.fruits field uses a field-level override order_by=["sweetness"],
and the test verifies that the field-level `order_by` on strawchemy.field takes
precedence; place this docstring above the imports so it documents the module,
mentioning the key symbols StrawchemyConfig, strawchemy, FruitType, and Query
for clarity.
| @strawchemy.type(Container, include="all", paginate=["vegetables"]) | ||
| class ContainerType: | ||
| pass | ||
|
|
||
|
|
||
| @strawberry.type | ||
| class Query: | ||
| container: list[ContainerType] = strawchemy.field() |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add documentation explaining type-level override behavior.
This test demonstrates an important precedence rule: type-level paginate=["vegetables"] overrides config-level pagination=["fruits"]. This override behavior should be documented to help users understand the configuration hierarchy.
📝 Suggested documentation
+"""Test schema demonstrating type-level pagination override.
+
+Config sets pagination=['fruits'], but ContainerType overrides with paginate=['vegetables'].
+Type-level settings take precedence over config-level settings.
+"""
+
from __future__ import annotationsCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @tests/unit/schemas/pagination/pagination_config_with_type_override.py around
lines 11 - 18, Add a brief doc note explaining that type-level paginate
overrides config-level pagination using this test as an example: mention that
the @strawchemy.type annotation on Container with paginate=["vegetables"] takes
precedence over any global/config setting like pagination=["fruits"], and point
to the ContainerType class and the Query.container field in the test as the
concrete example demonstrating this precedence; keep the note short, state the
precedence rule clearly, and include the exact symbol names paginate and
pagination so readers can map behavior to code.
1 participant
Description
Types of Changes
Issues Fixed or Closed by This PR
Checklist
Summary by CodeRabbit
Release Notes
New Features
Documentation
Breaking Changes
child_pagination→paginate,child_order_by→orderpagination="all"instead of boolean values✏️ Tip: You can customize this high-level summary in your review settings.