Skip to content

Fix empty list default for GraphQL list fields#2955

Closed
koxudaxi wants to merge 1 commit intomainfrom
fix/graphql-empty-list-default
Closed

Fix empty list default for GraphQL list fields#2955
koxudaxi wants to merge 1 commit intomainfrom
fix/graphql-empty-list-default

Conversation

@koxudaxi
Copy link
Copy Markdown
Owner

@koxudaxi koxudaxi commented Jan 10, 2026

Fixes: #2947

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced handling of empty list defaults when generating Pydantic models from GraphQL schemas. List fields with empty defaults now initialize with standard list factory methods instead of complex lambda expressions, resulting in cleaner and more efficient generated code.
  • Tests

    • Added test coverage for empty list default behavior in GraphQL model generation.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 10, 2026

📝 Walkthrough

Walkthrough

This PR fixes a bug where GraphQL list fields with empty list defaults [] were generating incorrect default factories that caused validation errors. The fix updates pydantic model generation to use default_factory=list for empty list defaults instead of a complex lambda that attempts to validate an empty list as a model instance.

Changes

Cohort / File(s) Summary
Core Logic Fix
src/datamodel_code_generator/model/pydantic/base_model.py
Added handling for single-item list fields with BaseModel type. When the default is an empty list, returns default_factory=list; otherwise returns a lambda that applies the parse method to each element. Preserves existing behavior for all other cases.
Test Data & Expectations
tests/data/expected/main/graphql/empty_list_default.py
New generated Pydantic model file showing expected output: ModelInput with tags field using Field(default_factory=list) for empty list defaults.
Test Coverage
tests/main/graphql/test_main_graphql.py
New test function verifying that empty list defaults generate Field(default_factory=list) instead of the problematic Field(default_factory=lambda: TagInput.model_validate([])).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A list that's empty, oh what a sight,
No lambda factories causing such fright!
With default_factory=list we now stand tall,
Models validate cleanly, zero errors at all! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main change: fixing the default factory behavior for empty list defaults in GraphQL list fields.
Linked Issues check ✅ Passed The PR directly addresses issue #2947 by fixing the default_factory generation for GraphQL list fields with empty list defaults, implementing the expected behavior of Field(default_factory=list).
Out of Scope Changes check ✅ Passed All changes are focused on fixing the GraphQL empty list default issue: the core logic fix in base_model.py, the expected test output file, and the corresponding test case.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

📚 Docs Preview: https://pr-2955.datamodel-code-generator.pages.dev

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
src/datamodel_code_generator/model/pydantic/base_model.py (2)

129-129: Remove the unused noqa directive.

The rules PLR0911 and PLR0912 are not enabled, making this directive unnecessary.

🧹 Proposed fix
-    def _get_default_as_pydantic_model(self) -> str | None:  # noqa: PLR0911, PLR0912
+    def _get_default_as_pydantic_model(self) -> str | None:

132-163: Consider refactoring to eliminate code duplication.

The new handling path (lines 132-145) and the existing loop logic (lines 151-163) contain nearly identical code for processing list types with BaseModel elements. The main difference is that the new code processes self.data_type directly while the loop processes members of self.data_type.data_types.

While the current implementation correctly fixes the empty list default issue, consider extracting the shared logic into a helper method to improve maintainability.

♻️ Proposed refactoring approach

Extract the list processing logic into a helper method:

def _get_list_default_factory(self, data_type_child, default):
    """Helper to generate default_factory for list of BaseModel types."""
    if not default:
        return STANDARD_LIST
    return (
        f"lambda :[{data_type_child.alias or data_type_child.reference.source.class_name}."
        f"{self._PARSE_METHOD}(v) for v in {default!r}]"
    )

Then use it in both locations:

# Lines 132-145
if self.data_type.is_list and len(self.data_type.data_types) == 1:
    data_type_child = self.data_type.data_types[0]
    if (
        data_type_child.reference
        and isinstance(data_type_child.reference.source, BaseModelBase)
        and isinstance(self.default, list)
    ):
        return self._get_list_default_factory(data_type_child, self.default)

# Lines 151-163
if data_type.is_list and len(data_type.data_types) == 1:
    data_type_child = data_type.data_types[0]
    if (
        data_type_child.reference
        and isinstance(data_type_child.reference.source, BaseModelBase)
        and isinstance(self.default, list)
    ):
        return self._get_list_default_factory(data_type_child, self.default)
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fa1fc11 and 99574c6.

⛔ Files ignored due to path filters (1)
  • tests/data/graphql/empty_list_default.graphql is excluded by !tests/data/**/*.graphql and included by none
📒 Files selected for processing (3)
  • src/datamodel_code_generator/model/pydantic/base_model.py
  • tests/data/expected/main/graphql/empty_list_default.py
  • tests/main/graphql/test_main_graphql.py
🧰 Additional context used
🧬 Code graph analysis (2)
tests/main/graphql/test_main_graphql.py (1)
tests/main/conftest.py (2)
  • output_file (99-101)
  • run_main_and_assert (245-409)
src/datamodel_code_generator/model/pydantic/base_model.py (4)
src/datamodel_code_generator/model/_types.py (1)
  • WrappedDefault (10-18)
src/datamodel_code_generator/parser/base.py (1)
  • data_type (1175-1177)
src/datamodel_code_generator/reference.py (1)
  • reference (77-79)
src/datamodel_code_generator/model/base.py (2)
  • class_name (860-862)
  • class_name (865-869)
🪛 Ruff (0.14.10)
src/datamodel_code_generator/model/pydantic/base_model.py

129-129: Unused noqa directive (non-enabled: PLR0911, PLR0912)

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). (13)
  • GitHub Check: 3.10 on Windows
  • GitHub Check: 3.12 on Windows
  • GitHub Check: 3.12 on macOS
  • GitHub Check: 3.14 on macOS
  • GitHub Check: 3.13 on Windows
  • GitHub Check: 3.11 on macOS
  • GitHub Check: 3.11 on Windows
  • GitHub Check: 3.10 on macOS
  • GitHub Check: benchmarks
  • GitHub Check: 3.14 on Windows
  • GitHub Check: build-deploy
  • GitHub Check: docs
  • GitHub Check: Analyze (python)
🔇 Additional comments (3)
src/datamodel_code_generator/model/pydantic/base_model.py (1)

140-141: LGTM - Correct fix for empty list defaults.

Returning STANDARD_LIST for empty list defaults correctly resolves the issue where default_factory=lambda: NestedClass.model_validate([]) was causing validation errors. The generated code now properly uses Field(default_factory=list).

tests/main/graphql/test_main_graphql.py (1)

895-911: LGTM - Excellent test coverage for the fix.

This test properly verifies that empty list defaults in GraphQL schemas generate Field(default_factory=list) instead of the problematic Field(default_factory=lambda: TagInput.model_validate([])). The test is well-documented with a clear reference to issue #2947.

tests/data/expected/main/graphql/empty_list_default.py (1)

1-32: LGTM - Expected output correctly demonstrates the fix.

The generated model at line 31 properly uses Field(default_factory=list) for the empty list default, which is exactly the behavior this PR aims to achieve. This resolves the validation error that occurred when using lambda: TagInput.model_validate([]).

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Jan 10, 2026

Merging this PR will not alter performance

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

✅ 11 untouched benchmarks
⏩ 98 skipped benchmarks1


Comparing fix/graphql-empty-list-default (99574c6) with main (fa1fc11)

Open in CodSpeed

Footnotes

  1. 98 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 10, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (fa1fc11) to head (99574c6).

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #2955   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           94        94           
  Lines        17723     17730    +7     
  Branches      2037      2039    +2     
=========================================
+ Hits         17723     17730    +7     
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@koxudaxi koxudaxi closed this Jan 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unexpected default_factory for list of object field with optional [] default results in validation failures during instantiation

1 participant