Conversation
WalkthroughIntroduces StrawchemyFactories and MutationFieldBuilder, refactors mapper and aggregation factories to use centralized factories and type-filter configs, adds slotscheck tooling and task, updates lint/ruff/unasyncd config, CI install step, minor task/docs edits, removes two pyright-ignore comments, and adds Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used🧠 Learnings (7)📚 Learning: 2025-09-14T11:55:31.395ZApplied to files:
📚 Learning: 2025-09-14T11:55:31.395ZApplied to files:
📚 Learning: 2025-09-14T11:55:31.395ZApplied to files:
📚 Learning: 2025-09-14T11:55:31.395ZApplied to files:
📚 Learning: 2025-09-14T11:55:31.395ZApplied to files:
📚 Learning: 2025-09-14T11:55:31.395ZApplied to files:
📚 Learning: 2025-09-14T11:55:31.395ZApplied to files:
🪛 YAMLlint (1.37.1).github/workflows/ci.yaml[error] 9-9: too many spaces inside brackets (brackets) [error] 9-9: too many spaces inside brackets (brackets) [error] 13-13: too many spaces inside brackets (brackets) [error] 13-13: too many spaces inside brackets (brackets) [error] 60-60: too many spaces inside brackets (brackets) [error] 60-60: too many spaces inside brackets (brackets) [error] 124-124: too many spaces inside brackets (brackets) [error] 124-124: too many spaces inside brackets (brackets) [error] 158-158: too many spaces inside brackets (brackets) [error] 158-158: too many spaces inside brackets (brackets) ⏰ 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). (3)
🔇 Additional comments (5)
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: 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/strawberry/factories/aggregations.py (1)
240-336: Functional but repetitive pattern.The method works correctly, routing through the new
arguments_typemethod with the appropriate aggregation keys. However, the repetitive structure for each datetime/date/time/string variant could be consolidated using a data-driven approach similar to_aggregation_type_filters. This is optional since the current implementation is clear and explicit.🔎 Example consolidation approach
_MIN_MAX_FILTER_CONFIGS = [ ("min_max_numeric", float, None, None), # (key, comparison_type, min_suffix, max_suffix) ("min_max_datetime", datetime, "min_datetime", "max_datetime"), ("min_max_date", date, "min_date", "max_date"), ("min_max_time", time, "min_time", "max_time"), ("min_max_string", str, "min_string", "max_string"), ] def _min_max_filters(self, model: type[DeclarativeBase], dto_config: DTOConfig) -> list[FilterFunctionInfo]: aggregations: list[FilterFunctionInfo] = [] for key, comparison_type, min_name, max_name in self._MIN_MAX_FILTER_CONFIGS: if fields := self.arguments_type(model, dto_config, key): for func, name in [("min", min_name), ("max", max_name)]: kwargs = {"field_name_": name} if name else {} aggregations.append( FilterFunctionInfo( enum_fields=fields, function=func, aggregation_type=key if name else "numeric", comparison_type=self._inspector.get_type_comparison(comparison_type), **kwargs, ) ) return aggregations
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
.gitignore(1 hunks)mise.toml(3 hunks)pyproject.toml(2 hunks)src/strawchemy/factories.py(1 hunks)src/strawchemy/mapper.py(8 hunks)src/strawchemy/strawberry/factories/aggregations.py(11 hunks)src/strawchemy/strawberry/factories/inputs.py(1 hunks)src/strawchemy/strawberry/mutation/builder.py(1 hunks)tasks.md(14 hunks)tests/unit/mapping/test_schemas.py(1 hunks)tests/unit/test_mutation_input.py(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-14T11:55:31.395Z
Learnt from: CR
Repo: gazorby/strawchemy PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-09-14T11:55:31.395Z
Learning: Consult the tasks documentation at tasks.md for available tasks
Applied to files:
tasks.mdmise.toml
📚 Learning: 2025-09-14T11:55:31.395Z
Learnt from: CR
Repo: gazorby/strawchemy PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-09-14T11:55:31.395Z
Learning: Run `mise run lint` to execute linters and formatters (optional to run manually; will run via pre-commit)
Applied to files:
tasks.md
📚 Learning: 2025-09-14T11:55:31.395Z
Learnt from: CR
Repo: gazorby/strawchemy PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-09-14T11:55:31.395Z
Learning: Run `mise run install` to install all dependencies and pre-commit hooks
Applied to files:
mise.toml
🧬 Code graph analysis (5)
tests/unit/test_mutation_input.py (2)
tests/unit/schemas/mutation_and_query.py (1)
ColorInput(23-23)src/strawchemy/strawberry/mutation/input.py (1)
Input(276-385)
tests/unit/mapping/test_schemas.py (4)
tests/integration/models.py (1)
id(61-67)src/strawchemy/dto/base.py (1)
name(347-348)src/strawchemy/sqlalchemy/_query.py (2)
name(112-114)name(781-787)src/strawchemy/sqlalchemy/_scope.py (1)
name(369-383)
src/strawchemy/mapper.py (2)
src/strawchemy/factories.py (3)
StrawchemyFactories(24-135)create(54-114)create_public_api(116-135)src/strawchemy/strawberry/mutation/builder.py (2)
MutationFieldBuilder(28-109)build(39-109)
src/strawchemy/strawberry/mutation/builder.py (2)
src/strawchemy/config/base.py (1)
StrawchemyConfig(21-62)src/strawchemy/strawberry/_field.py (3)
StrawchemyCreateMutationField(507-530)StrawchemyUpdateMutationField(598-653)StrawchemyUpsertMutationField(533-595)
src/strawchemy/strawberry/factories/aggregations.py (7)
src/strawchemy/strawberry/factories/inputs.py (3)
iter_field_definitions(130-162)factory(171-198)factory(451-480)src/strawchemy/strawberry/factories/types.py (7)
iter_field_definitions(166-188)iter_field_definitions(269-303)iter_field_definitions(423-448)iter_field_definitions(630-647)factory(191-242)factory(306-335)factory(650-679)src/strawchemy/strawberry/factories/enum.py (1)
iter_field_definitions(125-136)src/strawchemy/strawberry/factories/base.py (3)
iter_field_definitions(329-346)factory(349-398)factory(517-546)src/strawchemy/strawberry/dto.py (9)
FunctionArgFieldDefinition(328-331)from_field(249-257)from_field(309-316)function(264-264)function(267-267)function(270-270)function(272-278)FilterFunctionInfo(212-225)OutputFunctionInfo(204-208)src/strawchemy/dto/types.py (1)
DTOConfig(105-257)src/strawchemy/dto/exceptions.py (1)
DTOError(6-6)
🪛 Ruff (0.14.8)
src/strawchemy/strawberry/mutation/builder.py
47-47: Dynamically typed expressions (typing.Any) are disallowed in resolver
(ANN401)
50-50: Dynamically typed expressions (typing.Any) are disallowed in graphql_type
(ANN401)
55-55: Dynamically typed expressions (typing.Any) are disallowed in default
(ANN401)
60-60: Dynamically typed expressions (typing.Any) are disallowed in **field_specific_kwargs
(ANN401)
61-61: Dynamically typed expressions (typing.Any) are disallowed in build
(ANN401)
src/strawchemy/factories.py
64-64: Unused noqa directive (non-enabled: PLC0415)
Remove unused noqa directive
(RUF100)
65-65: Unused noqa directive (non-enabled: PLC0415)
Remove unused noqa directive
(RUF100)
66-66: Unused noqa directive (non-enabled: PLC0415)
Remove unused noqa directive
(RUF100)
67-67: Unused noqa directive (non-enabled: PLC0415)
Remove unused noqa directive
(RUF100)
71-71: Unused noqa directive (non-enabled: PLC0415)
Remove unused noqa directive
(RUF100)
75-75: Unused noqa directive (non-enabled: PLC0415)
Remove unused noqa directive
(RUF100)
⏰ 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.13
- GitHub Check: 🔬 integration-sqlite on 3.14
- GitHub Check: 🔬 integration-mysql on 3.14
- GitHub Check: 🔬 unit-no-extras on 3.12
- GitHub Check: 🔬 unit on 3.13
- GitHub Check: 🔬 unit on 3.10
- GitHub Check: 🔬 integration-postgres on 3.12
- GitHub Check: 🔬 integration-mysql on 3.13
- GitHub Check: 🔬 integration-postgres on 3.13
- GitHub Check: 🔬 integration-sqlite on 3.12
- GitHub Check: 🔬 integration-postgres on 3.14
- GitHub Check: 🔬 unit on 3.11
- GitHub Check: 🔬 integration-mysql on 3.11
- GitHub Check: 🔬 unit-no-extras on 3.14
- GitHub Check: 🔬 unit-no-extras on 3.13
- GitHub Check: 🔬 unit on 3.12
- GitHub Check: 🔬 integration-postgres on 3.11
- GitHub Check: 🔬 unit-no-extras on 3.11
- GitHub Check: 🔬 unit on 3.14
- GitHub Check: Optimize new code in this PR
🔇 Additional comments (29)
.gitignore (1)
195-196: LGTM!Adding
.serenato the gitignore is appropriate for excluding tool-specific configuration from version control.tests/unit/test_mutation_input.py (1)
22-25: LGTM!Removing the Pyright type-ignore comments is a positive cleanup, indicating that the type system now correctly infers these types after the refactoring work in this PR.
tests/unit/mapping/test_schemas.py (1)
65-67: LGTM!Consistent with the cleanup in
test_mutation_input.py, removing type-ignore comments here indicates improved type inference for dynamically generated input types.src/strawchemy/strawberry/factories/inputs.py (1)
397-397: LGTM!Narrowing the type hint from
type[Any]totype[DeclarativeBase]is a good refinement that accurately reflects the expected SQLAlchemy model type and improves type safety.pyproject.toml (3)
380-396: LGTM!The slotscheck configuration is well-structured with appropriate exclusions for test files and external modules. The comment referencing
slotscheck/issues/178for the SQLAlchemy mypy exclusion is helpful documentation.
398-403: LGTM!The additional unasyncd options are sensible defaults that improve the quality of generated synchronous code by automatically applying formatting and updating docstrings.
66-66: LGTM!Adding
slotscheckto the lint dependencies is a good addition for verifying proper__slots__usage in the codebase. Version 0.16.5 and later are available on PyPI.mise.toml (2)
218-218: LGTM! Slotscheck task addition is well-structured.The new
slotschecktask is properly defined with a clear description and command, and correctly wired as a dependency of thelinttask. This enhances runtime memory efficiency checks for class definitions.Also applies to: 229-231
247-255: Formatting-only changes to clean task.These are cosmetic indentation adjustments with no semantic impact.
tasks.md (2)
73-73: LGTM! Documentation correctly reflects slotscheck task addition.The generated documentation is consistent with the
mise.tomlchanges.Also applies to: 132-136
151-152: Auto-generated default placeholders are correct.The empty
**Default:** `` entries throughout the file correctly represent optional arguments with empty string defaults, matching thearg(..., default="")specifications inmise.toml`.src/strawchemy/factories.py (3)
23-51: Well-designed factory container with clear responsibilities.The
StrawchemyFactoriesdataclass provides a clean centralized container for all DTO factories with comprehensive docstrings. The# type: ignore[type-arg]comments on lines 46-48 are acceptable for complex generic factory types where full type parameters would add noise without practical benefit.
53-114: Factory wiring logic is clean and well-documented.The
create()classmethod properly initializes all backends and factories with their dependencies. The dependency injection pattern here supports easier testing and better separation of concerns.
116-135: Public API mapping is clear and consistent.The
create_public_api()method provides a clean mapping of user-facing API names to internal factory methods, usingpartial()appropriately to bind mode parameters.src/strawchemy/mapper.py (5)
93-123: Excellent refactor to centralized factory management.The initialization now cleanly delegates to
StrawchemyFactories.create(self), storing private factory references for internal use while exposing the public API throughcreate_public_api(). This significantly improves testability and maintainability.
364-380: Clean delegation to MutationFieldBuilder.The
create()method now delegates to_mutation_builder.build(), eliminating boilerplate while maintaining the same functionality. Theinput_typeandvalidationare correctly passed as field-specific kwargs.
440-458: Upsert method correctly passes enum kwargs.The
update_fields_enumandconflict_fields_enumare properly forwarded to the builder as field-specific kwargs, matching theStrawchemyUpsertMutationFieldconstructor expectations.
516-533: Update method correctly forwards filter_type.The
filter_inputis passed asfilter_typeto the builder, consistent with the field class's expected parameter naming.
660-675: Delete method correctly handles optional filter_input.The
filter_inputis passed asinput_typeto the builder.StrawchemyDeleteMutationFieldhandlesNonegracefully—itsauto_arguments()method checksif self._input_type:before creating filter arguments, returning an empty list whenNone.src/strawchemy/strawberry/mutation/builder.py (3)
27-37: Well-structured builder dataclass.The
MutationFieldBuildercleanly encapsulates the configuration and namespace getter needed for mutation field construction. Using a dataclass provides immutability for the configuration while supporting the builder pattern.
39-61:Anytypes are appropriate for this dynamic builder interface.The static analysis flags
ANN401for severalAnyusages, but these are justified:
resolver: Accepts any callable signaturegraphql_type: Strawberry types are dynamically resolveddefault: Mirrorsdataclasses.MISSINGpattern**field_specific_kwargs: Must accept various field-specific parameters- Return type: Returns different mutation field types
This is a valid use of
Anyfor a generic factory/builder pattern.
87-109: Build logic correctly delegates to field constructors.The method properly:
- Resolves the namespace from the getter
- Creates
StrawberryAnnotationonly whengraphql_typeis provided- Falls back to
config.repository_typewhenrepository_typeisNone- Passes all common and field-specific kwargs to the field constructor
- Wraps with resolver if provided
This eliminates significant duplication across mutation methods in the mapper.
src/strawchemy/strawberry/factories/aggregations.py (7)
41-52: Well-designed configuration dataclass.The
_TypeFilterConfigdataclass is appropriately frozen and slotted for immutability and memory efficiency. Usingfrozensetfor the types collection is a good choice since these shouldn't be mutated after creation.
105-112: LGTM!The variable rename from
fieldtofield_defimproves clarity and avoids shadowing the built-in. The conditional wrapping withFunctionArgFieldDefinition.from_fieldwhen a function is provided aligns with thefrom_fieldsignature in the relevant code snippets.
208-228: Clean configuration-driven factory initialization.The centralized
_aggregation_type_filtersclass variable provides a clear, maintainable way to define aggregation type configurations. The instance-level_type_filtered_factoriesdictionary is appropriately created per inspector since factories hold mapper references. This pattern makes adding new aggregation types straightforward.
338-348: LGTM!The method properly handles missing aggregation keys with a safe
.get()lookup and returnsNonefor both unknown keys and when no matching fields exist (viaDTOErrorhandling).
350-378: Verify direct dictionary access is intentional.These methods use direct dictionary access (
_type_filtered_factories["key"]) whilearguments_typeuses.get(). The direct access is safe since the keys are hardcoded and guaranteed to exist in_aggregation_type_filters. However, if you prefer consistency witharguments_type, consider using.get()with a fallback. The current approach is valid as it will fail fast if configuration is inadvertently broken.
380-412: LGTM!The method signature is updated to use
type[DeclarativeBase]consistently with the rest of the refactor. The aggregation logic remains sound and properly integrates with the updated factory methods.
414-457: LGTM!The method properly uses the new
arguments_typemethod for factory lookup and maintains the existing aggregation filter logic. The type hint update is consistent with the broader refactor.
| from strawchemy.dto.backend.strawberry import StrawberrryDTOBackend # noqa: PLC0415 | ||
| from strawchemy.strawberry.dto import MappedStrawberryGraphQLDTO # noqa: PLC0415 | ||
| from strawchemy.strawberry.factories.aggregations import EnumDTOFactory # noqa: PLC0415 | ||
| from strawchemy.strawberry.factories.enum import ( # noqa: PLC0415 | ||
| EnumDTOBackend, | ||
| UpsertConflictFieldsEnumDTOBackend, | ||
| ) | ||
| from strawchemy.strawberry.factories.inputs import ( # noqa: PLC0415 | ||
| AggregateFilterDTOFactory, | ||
| BooleanFilterDTOFactory, | ||
| ) | ||
| from strawchemy.strawberry.factories.types import ( # noqa: PLC0415 | ||
| DistinctOnFieldsDTOFactory, | ||
| InputFactory, | ||
| OrderByDTOFactory, | ||
| RootAggregateTypeDTOFactory, | ||
| TypeDTOFactory, | ||
| UpsertConflictFieldsDTOFactory, | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove unused noqa directives.
Static analysis indicates that PLC0415 is not enabled in your ruff configuration, making these noqa comments unnecessary. The internal imports are still valid and intentional for avoiding circular dependencies.
🔎 Proposed fix
- from strawchemy.dto.backend.strawberry import StrawberrryDTOBackend # noqa: PLC0415
- from strawchemy.strawberry.dto import MappedStrawberryGraphQLDTO # noqa: PLC0415
- from strawchemy.strawberry.factories.aggregations import EnumDTOFactory # noqa: PLC0415
- from strawchemy.strawberry.factories.enum import ( # noqa: PLC0415
+ from strawchemy.dto.backend.strawberry import StrawberrryDTOBackend
+ from strawchemy.strawberry.dto import MappedStrawberryGraphQLDTO
+ from strawchemy.strawberry.factories.aggregations import EnumDTOFactory
+ from strawchemy.strawberry.factories.enum import (
EnumDTOBackend,
UpsertConflictFieldsEnumDTOBackend,
)
- from strawchemy.strawberry.factories.inputs import ( # noqa: PLC0415
+ from strawchemy.strawberry.factories.inputs import (
AggregateFilterDTOFactory,
BooleanFilterDTOFactory,
)
- from strawchemy.strawberry.factories.types import ( # noqa: PLC0415
+ from strawchemy.strawberry.factories.types import (
DistinctOnFieldsDTOFactory,
InputFactory,
OrderByDTOFactory,
RootAggregateTypeDTOFactory,
TypeDTOFactory,
UpsertConflictFieldsDTOFactory,
)📝 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 strawchemy.dto.backend.strawberry import StrawberrryDTOBackend # noqa: PLC0415 | |
| from strawchemy.strawberry.dto import MappedStrawberryGraphQLDTO # noqa: PLC0415 | |
| from strawchemy.strawberry.factories.aggregations import EnumDTOFactory # noqa: PLC0415 | |
| from strawchemy.strawberry.factories.enum import ( # noqa: PLC0415 | |
| EnumDTOBackend, | |
| UpsertConflictFieldsEnumDTOBackend, | |
| ) | |
| from strawchemy.strawberry.factories.inputs import ( # noqa: PLC0415 | |
| AggregateFilterDTOFactory, | |
| BooleanFilterDTOFactory, | |
| ) | |
| from strawchemy.strawberry.factories.types import ( # noqa: PLC0415 | |
| DistinctOnFieldsDTOFactory, | |
| InputFactory, | |
| OrderByDTOFactory, | |
| RootAggregateTypeDTOFactory, | |
| TypeDTOFactory, | |
| UpsertConflictFieldsDTOFactory, | |
| ) | |
| from strawchemy.dto.backend.strawberry import StrawberrryDTOBackend | |
| from strawchemy.strawberry.dto import MappedStrawberryGraphQLDTO | |
| from strawchemy.strawberry.factories.aggregations import EnumDTOFactory | |
| from strawchemy.strawberry.factories.enum import ( | |
| EnumDTOBackend, | |
| UpsertConflictFieldsEnumDTOBackend, | |
| ) | |
| from strawchemy.strawberry.factories.inputs import ( | |
| AggregateFilterDTOFactory, | |
| BooleanFilterDTOFactory, | |
| ) | |
| from strawchemy.strawberry.factories.types import ( | |
| DistinctOnFieldsDTOFactory, | |
| InputFactory, | |
| OrderByDTOFactory, | |
| RootAggregateTypeDTOFactory, | |
| TypeDTOFactory, | |
| UpsertConflictFieldsDTOFactory, | |
| ) |
🧰 Tools
🪛 Ruff (0.14.8)
64-64: Unused noqa directive (non-enabled: PLC0415)
Remove unused noqa directive
(RUF100)
65-65: Unused noqa directive (non-enabled: PLC0415)
Remove unused noqa directive
(RUF100)
66-66: Unused noqa directive (non-enabled: PLC0415)
Remove unused noqa directive
(RUF100)
67-67: Unused noqa directive (non-enabled: PLC0415)
Remove unused noqa directive
(RUF100)
71-71: Unused noqa directive (non-enabled: PLC0415)
Remove unused noqa directive
(RUF100)
75-75: Unused noqa directive (non-enabled: PLC0415)
Remove unused noqa directive
(RUF100)
🤖 Prompt for AI Agents
In src/strawchemy/factories.py around lines 64 to 82, multiple import lines
include unnecessary "# noqa: PLC0415" directives; remove these unused noqa
comments from each import (e.g., after StrawberrryDTOBackend,
MappedStrawberryGraphQLDTO, EnumDTOFactory, EnumDTOBackend,
UpsertConflictFieldsEnumDTOBackend, AggregateFilterDTOFactory,
BooleanFilterDTOFactory, DistinctOnFieldsDTOFactory, InputFactory,
OrderByDTOFactory, RootAggregateTypeDTOFactory, TypeDTOFactory,
UpsertConflictFieldsDTOFactory) so the imports remain but the redundant noqa
annotations are deleted.
| def __init__( | ||
| self, | ||
| base_name: str, | ||
| dto_config: DTOConfig, | ||
| node: Node[Relation[Any, UnmappedStrawberryGraphQLDTO[ModelT]], None] | None = None, | ||
| ) -> str: | ||
| return f"{base_name}MinMaxStringFields" | ||
|
|
||
|
|
||
| class _MinMaxTimeFieldsDTOFactory(_FunctionArgDTOFactory): | ||
| types: ClassVar[set[type[Any]]] = {time} | ||
| mapper: Strawchemy, | ||
| filter_config: _TypeFilterConfig, | ||
| backend: DTOBackend[UnmappedStrawberryGraphQLDTO[DeclarativeBase]] | None = None, | ||
| ) -> None: | ||
| super().__init__(mapper, backend) | ||
| self._filter_types = set(filter_config.types) | ||
| self._suffix = filter_config.suffix |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider keeping _filter_types as frozenset instead of converting to set.
Since _filter_types is only used for membership testing (not in self._filter_types), there's no need to convert the immutable frozenset to a mutable set. Keeping it as frozenset maintains consistency with the source config and prevents accidental mutation.
🔎 Proposed fix
def __init__(
self,
mapper: Strawchemy,
filter_config: _TypeFilterConfig,
backend: DTOBackend[UnmappedStrawberryGraphQLDTO[DeclarativeBase]] | None = None,
) -> None:
super().__init__(mapper, backend)
- self._filter_types = set(filter_config.types)
+ self._filter_types = filter_config.types
self._suffix = filter_config.suffix🤖 Prompt for AI Agents
In src/strawchemy/strawberry/factories/aggregations.py around lines 174 to 182,
the constructor converts filter_config.types into a mutable set even though only
membership checks are performed; change self._filter_types to hold the original
frozenset (or cast to frozenset) instead of set to avoid unnecessary mutability
and preserve the config's immutability — replace the set(...) conversion with
using filter_config.types directly (or wrap with frozenset(...) if type
assurance is needed).
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yaml(5 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-14T11:55:31.395Z
Learnt from: CR
Repo: gazorby/strawchemy PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-09-14T11:55:31.395Z
Learning: Run `mise run lint` to execute linters and formatters (optional to run manually; will run via pre-commit)
Applied to files:
.github/workflows/ci.yaml
📚 Learning: 2025-09-14T11:55:31.395Z
Learnt from: CR
Repo: gazorby/strawchemy PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-09-14T11:55:31.395Z
Learning: Run `mise run install` to install all dependencies and pre-commit hooks
Applied to files:
.github/workflows/ci.yaml
📚 Learning: 2025-09-14T11:55:31.395Z
Learnt from: CR
Repo: gazorby/strawchemy PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-09-14T11:55:31.395Z
Learning: Use mise to manage build and test tasks for the project
Applied to files:
.github/workflows/ci.yaml
🪛 YAMLlint (1.37.1)
.github/workflows/ci.yaml
[error] 9-9: too many spaces inside brackets
(brackets)
[error] 9-9: too many spaces inside brackets
(brackets)
[error] 13-13: too many spaces inside brackets
(brackets)
[error] 13-13: too many spaces inside brackets
(brackets)
[error] 60-60: too many spaces inside brackets
(brackets)
[error] 60-60: too many spaces inside brackets
(brackets)
[error] 124-124: too many spaces inside brackets
(brackets)
[error] 124-124: too many spaces inside brackets
(brackets)
[error] 158-158: too many spaces inside brackets
(brackets)
[error] 158-158: too many spaces inside brackets
(brackets)
⏰ 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). (7)
- GitHub Check: 🔬 integration-sqlite on 3.10
- GitHub Check: 🔬 unit-no-extras on 3.13
- GitHub Check: 🔬 integration-postgres on 3.13
- GitHub Check: 🔬 unit-no-extras on 3.12
- GitHub Check: 🔬 unit-no-extras on 3.14
- GitHub Check: ✨ Lint
- GitHub Check: Optimize new code in this PR
🔇 Additional comments (1)
.github/workflows/ci.yaml (1)
199-201: Good addition to ensure dependencies are installed before linting.This step properly prepares the lint environment by installing dependencies (including the newly added slotscheck tooling). This aligns well with the project's use of mise for task management.
| on: | ||
| push: | ||
| branches: [main, ci/*, dependabot/*, renovate/*] | ||
| branches: [ main, ci/*, dependabot/*, renovate/* ] |
There was a problem hiding this comment.
Remove extra spaces inside brackets to comply with YAML style guidelines.
YAMLlint flags "too many spaces inside brackets" on these lines. The standard YAML convention is to write arrays without spaces after [ and before ].
🔎 Proposed fix
- branches: [ main, ci/*, dependabot/*, renovate/* ]
+ branches: [main, ci/*, dependabot/*, renovate/*]- branches: [ main ]
+ branches: [main]- needs: [ pre, generate-jobs-tests ]
+ needs: [pre, generate-jobs-tests]- needs: [ tests, generate-jobs-tests ]
+ needs: [tests, generate-jobs-tests]- needs: [ tests, generate-jobs-tests ]
+ needs: [tests, generate-jobs-tests]Also applies to: 13-13, 60-60, 124-124, 158-158
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 9-9: too many spaces inside brackets
(brackets)
[error] 9-9: too many spaces inside brackets
(brackets)
🤖 Prompt for AI Agents
.github/workflows/ci.yaml lines 9, 13, 60, 124 and 158: the array entries use
spaces inside square brackets (e.g. "[ main, ci/*, ... ]") which violates YAML
lint rules; remove the extra spaces immediately after '[' and before ']' so
arrays read like "[main, ci/*, dependabot/*, renovate/*]" (and similarly for the
other lines) to comply with YAML style guidelines.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #138 +/- ##
==========================================
- Coverage 94.37% 94.29% -0.09%
==========================================
Files 63 65 +2
Lines 5833 5832 -1
Branches 775 770 -5
==========================================
- Hits 5505 5499 -6
- Misses 186 191 +5
Partials 142 142 ☔ View full report in Codecov by Sentry. |
1 participant
Description
Types of Changes
Issues Fixed or Closed by This PR
Checklist
Summary by CodeRabbit
Refactor
Chores
Tests
Style
✏️ Tip: You can customize this high-level summary in your review settings.