Add --default-values CLI option for overriding field defaults#2897
Add --default-values CLI option for overriding field defaults#2897
Conversation
|
Warning Rate limit exceeded@koxudaxi has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 2 minutes and 34 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughAdds a Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as __main__.py
participant FS as File I/O
participant Config
participant Parser as Parser (GraphQL/JSONSchema/OpenAPI)
participant Resolver as ModelResolver
User->>CLI: invoke CLI with --default-values file.json
CLI->>FS: read file.json
FS-->>CLI: raw JSON
CLI->>CLI: validate JSON is object (_load_json_config)
alt invalid JSON or non-object
CLI->>User: print error and exit
else valid mapping
CLI->>Config: attach default_value_overrides
CLI->>Parser: init parsers with config (includes overrides)
Parser->>Resolver: pass default_value_overrides into ModelResolver
loop for each field
Parser->>Resolver: resolve_default_value(field_name, original_default, has_default, class_name)
Resolver-->>Parser: effective_default, effective_has_default
Parser->>Parser: apply effective values to DataModelField (may set required=False)
end
Parser->>User: emit generated code with applied defaults
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
…s-override # Conflicts: # tests/main/jsonschema/test_main_jsonschema.py # tests/main/openapi/test_main_openapi.py
🤖 Generated by GitHub Actions
|
📚 Docs Preview: https://pr-2897.datamodel-code-generator.pages.dev |
CodSpeed Performance ReportMerging #2897 will not alter performanceComparing
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2897 +/- ##
=======================================
Coverage 99.37% 99.38%
=======================================
Files 92 92
Lines 16268 16335 +67
Branches 1921 1932 +11
=======================================
+ Hits 16167 16234 +67
Misses 52 52
Partials 49 49
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
docs/cli-reference/field-customization.md (1)
561-628: Consider documenting JSON mapping shape and precedence rulesThe
--default-valuessection is accurate but a bit terse compared to the CLI help. To make the feature easier to adopt, I’d suggest (optionally) adding:
- A small JSON snippet showing flat vs scoped keys (e.g.
{"User.status": "active", "page": 1}).- A short note that scoped keys override flat ones and that required fields stay required unless
--use-defaultis set.This would align the docs with the CLI help text and avoid ambiguity about how overrides are resolved.
src/datamodel_code_generator/parser/jsonschema.py (1)
1044-1046: Consider simplifying redundant conditional logic.The condition
if effective_has_default is not Noneis alwaysTruewhen called fromparse_object_fields(line 2422-2427), sinceresolve_default_valuealways returns a boolean for the second tuple element. The logic works correctly becauseresolve_default_valuereturns the original values when no override exists, but the conditional check is misleading.Consider either:
- Removing the conditional if all callers always provide these parameters
- Documenting that the parameters are optional for backward compatibility
- Using a sentinel value to explicitly distinguish "no override attempted" from "override returned original value"
🔎 Verify all callers provide effective values
#!/bin/bash # Find all calls to get_object_field to check if effective parameters are always provided ast-grep --pattern $'$_.get_object_field( $$$ )'tests/main/jsonschema/test_main_jsonschema.py (1)
7461-7483: New--default-valuesCLI-doc test is wired correctly; consider adding related options metadataThe cli-doc metadata, schema/golden paths, and
run_main_and_assertusage all follow the established pattern (mirroring the aliases tests), and the override file is correctly referenced viaDEFAULT_VALUES_DATA_PATH. You might optionally addrelated_options=["--use-default"](and/or other interacting flags) to the@pytest.mark.cli_docfor better cross-linking in the CLI docs, but the test itself looks solid.src/datamodel_code_generator/parser/openapi.py (1)
505-635: Default override + required/nullable handling inparse_all_parametersis consistent with JsonSchema behaviorThe new
effective_default/effective_has_defaulthandling for bothschema_andcontentbranches, plus recomputingeffective_requiredwhenapply_default_values_for_required_fieldsis enabled, matches the JsonSchema path and gives the right behavior for required parameters gaining defaults (including via overrides). The updated nullable computation in thecontentbranch usingeffective_has_default/effective_requiredalso stays in sync withget_object_field’s strict-nullable semantics. This overall wiring looks solid.Clean up unused
noqaonparse_all_parametersRuff reports
# noqa: PLR0912, PLR0914as unused for this function because those checks aren’t enabled; removing the directive avoids the RUF100 warning without affecting lint behavior.Proposed diff to drop the unused
noqa- def parse_all_parameters( # noqa: PLR0912, PLR0914 + def parse_all_parameters( self, name: str, parameters: list[ReferenceObject | ParameterObject], path: list[str], ) -> DataType | None:Verify class-scoped override key choice for parameters
One subtle point: aliases for parameters are resolved with
class_name=nameinget_valid_field_name_and_alias, while default overrides are resolved withclass_name=reference.nameinresolve_default_value. If users rely on class-scoped keys (e.g.MyParams.foo) for both aliases and default overrides, this split may require them to use different class names for the two features ifreference.nameever diverges fromnamedue to uniquification.If the intended contract is “use the generated Python class name” for both aliases and default overrides, it might be worth aligning the
class_nameused in alias resolution withreference.name; if the intended contract is instead “use the logical base name from the spec,” then keeping them different is correct and should be documented.src/datamodel_code_generator/parser/graphql.py (1)
432-443: Passing original field names into override/alias resolution inparse_object_likeis a good improvementIterating as
for original_field_name, field in obj.fields.items()and then:
- feeding
original_field_nameintoget_valid_field_name_and_aliaswithclass_name=obj.name, and- passing both
original_field_nameandclass_name=obj.nameintoparse_fieldmakes scoped aliasing and default overrides behave predictably for GraphQL types. The overall change reads clean and keeps the
exclude_field_nameslogic unmodified.If you ever decide you want
DataModelFieldBase.original_nameto consistently reflect the source schema name (here, the GraphQL field), you could consider switchingoriginal_name=field_nametooriginal_name=original_field_nameinparse_field. That would be a behavior change, though, so only do it if it aligns with how other parsers useoriginal_name.src/datamodel_code_generator/reference.py (1)
1199-1237: Remove unusednoqadirective.The logic for hierarchical default value resolution is well-structured and correctly implements scoped (ClassName.field_name) and flat (field_name) override resolution. However, the
# noqa: FBT001directive on line 1203 appears to be unused according to static analysis.🔎 Proposed fix
field_name: str, original_default: Any, - has_default: bool, # noqa: FBT001 + has_default: bool, class_name: str | None = None, ) -> tuple[Any, bool]:src/datamodel_code_generator/__main__.py (2)
506-506: Remove unusednoqadirective.The field definition is correct, but the
# noqa: UP045directive is flagged as unused by static analysis.🔎 Proposed fix
- default_values: Optional[TextIOBase] = None # noqa: UP045 + default_values: Optional[TextIOBase] = None
1196-1212: Clean up unusednoqadirectives.The validation logic correctly mirrors the pattern used for
aliasesand provides clear error messages. However, static analysis flags several unused# noqa: T201directives.🔎 Proposed fix
try: default_value_overrides = json.load(data) except json.JSONDecodeError as e: - print(f"Unable to load default values mapping: {e}", file=sys.stderr) # noqa: T201 + print(f"Unable to load default values mapping: {e}", file=sys.stderr) return Exit.ERROR if not isinstance(default_value_overrides, dict): - print("Unable to load default values mapping: must be a JSON object", file=sys.stderr) # noqa: T201 + print("Unable to load default values mapping: must be a JSON object", file=sys.stderr) return Exit.ERROR if not all(isinstance(k, str) for k in default_value_overrides): # pragma: no cover - print("Unable to load default values mapping: all keys must be strings", file=sys.stderr) # noqa: T201 + print("Unable to load default values mapping: all keys must be strings", file=sys.stderr) return Exit.ERROR # pragma: no cover
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
tests/data/default_values/graphql_user_defaults.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/default_values/invalid_json.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/default_values/non_dict.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/default_values/openapi_params_defaults.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/default_values/scoped_defaults.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/graphql/default_values_required.graphqlis excluded by!tests/data/**/*.graphqland included by nonetests/data/jsonschema/default_values_override.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/openapi/default_values_parameters.yamlis excluded by!tests/data/**/*.yamland included by none
📒 Files selected for processing (26)
docs/cli-reference/field-customization.mddocs/cli-reference/index.mddocs/cli-reference/quick-reference.mdsrc/datamodel_code_generator/__main__.pysrc/datamodel_code_generator/_types/generate_config_dict.pysrc/datamodel_code_generator/_types/parser_config_dicts.pysrc/datamodel_code_generator/arguments.pysrc/datamodel_code_generator/cli_options.pysrc/datamodel_code_generator/config.pysrc/datamodel_code_generator/parser/base.pysrc/datamodel_code_generator/parser/graphql.pysrc/datamodel_code_generator/parser/jsonschema.pysrc/datamodel_code_generator/parser/openapi.pysrc/datamodel_code_generator/prompt_data.pysrc/datamodel_code_generator/reference.pysrc/datamodel_code_generator/watch.pytests/data/expected/main/graphql/default_values_required_use_default.pytests/data/expected/main/input_model/config_class.pytests/data/expected/main/jsonschema/jsonschema_default_values_override.pytests/data/expected/main/openapi/default_values_parameters_use_default.pytests/main/conftest.pytests/main/graphql/test_main_graphql.pytests/main/jsonschema/test_main_jsonschema.pytests/main/openapi/test_main_openapi.pytests/main/test_main_general.pytests/main/test_public_api_signature_baseline.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-02T08:25:19.839Z
Learnt from: koxudaxi
Repo: koxudaxi/datamodel-code-generator PR: 2890
File: tests/data/expected/main/jsonschema/ref_nullable_with_constraint.py:14-15
Timestamp: 2026-01-02T08:25:19.839Z
Learning: The datamodel-code-generator currently generates RootModel subclasses with an explicit `root` field annotation (e.g., `class StringType(RootModel[str]): root: str`). This is existing behavior of the code generator and should not be flagged as an issue introduced by new changes.
Applied to files:
tests/data/expected/main/input_model/config_class.pytests/data/expected/main/jsonschema/jsonschema_default_values_override.py
🧬 Code graph analysis (10)
tests/main/jsonschema/test_main_jsonschema.py (1)
tests/main/conftest.py (1)
run_main_and_assert(245-409)
tests/main/graphql/test_main_graphql.py (1)
tests/main/conftest.py (1)
run_main_and_assert(245-409)
tests/main/openapi/test_main_openapi.py (1)
tests/main/conftest.py (2)
output_file(99-101)run_main_and_assert(245-409)
src/datamodel_code_generator/reference.py (2)
src/datamodel_code_generator/parser/jsonschema.py (1)
has_default(422-424)src/datamodel_code_generator/model/base.py (2)
class_name(848-850)class_name(853-857)
src/datamodel_code_generator/parser/jsonschema.py (2)
src/datamodel_code_generator/model/base.py (4)
DataModelFieldBase(151-469)field(404-406)class_name(848-850)class_name(853-857)src/datamodel_code_generator/reference.py (1)
resolve_default_value(1199-1236)
src/datamodel_code_generator/parser/openapi.py (2)
src/datamodel_code_generator/reference.py (2)
resolve_default_value(1199-1236)reference(77-79)src/datamodel_code_generator/parser/jsonschema.py (2)
has_default(422-424)get_object_field(1031-1088)
tests/data/expected/main/jsonschema/jsonschema_default_values_override.py (3)
tests/data/expected/main/graphql/default_values_required_use_default.py (1)
User(29-33)tests/data/expected/main/openapi/default_values_parameters_use_default.py (1)
User(19-21)src/datamodel_code_generator/model/base.py (1)
name(827-829)
tests/data/expected/main/graphql/default_values_required_use_default.py (2)
src/datamodel_code_generator/model/type_alias.py (1)
TypeAlias(37-42)tests/data/expected/main/jsonschema/jsonschema_default_values_override.py (1)
User(16-19)
src/datamodel_code_generator/__main__.py (2)
src/datamodel_code_generator/util.py (1)
field_validator(175-191)src/datamodel_code_generator/watch.py (1)
watch_and_regenerate(23-67)
src/datamodel_code_generator/parser/graphql.py (1)
src/datamodel_code_generator/reference.py (3)
resolve_default_value(1199-1236)get_valid_field_name_and_alias(304-354)get_valid_field_name_and_alias(1171-1197)
🪛 Ruff (0.14.10)
src/datamodel_code_generator/reference.py
1203-1203: Unused noqa directive (non-enabled: FBT001)
Remove unused noqa directive
(RUF100)
src/datamodel_code_generator/parser/openapi.py
505-505: Unused noqa directive (non-enabled: PLR0912, PLR0914)
Remove unused noqa directive
(RUF100)
src/datamodel_code_generator/__main__.py
506-506: Unused noqa directive (non-enabled: UP045)
Remove unused noqa directive
(RUF100)
1204-1204: Unused noqa directive (non-enabled: T201)
Remove unused noqa directive
(RUF100)
1207-1207: Unused noqa directive (non-enabled: T201)
Remove unused noqa directive
(RUF100)
1210-1210: Unused noqa directive (non-enabled: T201)
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). (17)
- GitHub Check: 3.10 on macOS
- GitHub Check: py312-isort5 on Ubuntu
- GitHub Check: py312-black22 on Ubuntu
- GitHub Check: py312-isort7 on Ubuntu
- GitHub Check: py312-black24 on Ubuntu
- GitHub Check: py312-isort6 on Ubuntu
- GitHub Check: 3.10 on Ubuntu
- GitHub Check: 3.11 on Ubuntu
- GitHub Check: 3.12 on macOS
- GitHub Check: 3.13 on macOS
- GitHub Check: 3.10 on Windows
- GitHub Check: 3.13 on Windows
- GitHub Check: 3.14 on Windows
- GitHub Check: 3.11 on macOS
- GitHub Check: 3.12 on Windows
- GitHub Check: benchmarks
- GitHub Check: Analyze (python)
🔇 Additional comments (36)
tests/data/expected/main/input_model/config_class.py (1)
241-241: LGTM! Auto-generated field addition is correct.The new
default_value_overridesfield follows the established pattern for optional configuration fields in theGenerateConfigTypedDict. The typeNotRequired[Mapping[str, Any] | None]appropriately allows omission,None, or a mapping structure for field default value overrides.Note: This file is auto-generated by datamodel-codegen itself (as indicated by the header), so the actual source for this change is in the generator's configuration schema.
src/datamodel_code_generator/prompt_data.py (1)
36-36: LGTM! Auto-generated documentation entry is clear and accurate.The description "Override field default values from external JSON file" is concise and accurately describes the
--default-valuesoption's purpose.Note: This file is auto-generated (header specifies: "DO NOT EDIT MANUALLY. Run: python scripts/build_prompt_data.py"), so the actual source is in the CLI documentation markers elsewhere in the codebase.
tests/data/expected/main/graphql/default_values_required_use_default.py (1)
1-33: LGTM! Test expectation file demonstrates the feature correctly.This generated output properly demonstrates the default value override feature in action. Line 32 shows the
statusfield with the overridden default value'active', applied to an optional field (String | None = 'active'). The GraphQL scalar type aliases and model structure follow the expected patterns for GraphQL schema code generation.tests/main/conftest.py (1)
71-71: LGTM! Test data path constant follows established conventions.The new
DEFAULT_VALUES_DATA_PATHconstant follows the existing pattern for test data paths (consistent naming, type annotation, and placement alongside similar constants likeALIASES_DATA_PATH). This will enable test files to consistently reference the default-values test data directory.docs/cli-reference/quick-reference.md (2)
65-65: LGTM! Documentation entry is correctly categorized and formatted.The
--default-valuesoption is appropriately placed in the Field Customization section with a clear description and proper link to the detailed documentation.
230-230: LGTM! Alphabetical index entry is correctly positioned.The entry is properly placed alphabetically (between
--dataclass-argumentsand--disable-appending-item-suffix) and uses consistent formatting with the other index entries.src/datamodel_code_generator/_types/generate_config_dict.py (1)
166-166: LGTM!The new
default_value_overridesfield is correctly typed and properly marked as optional usingNotRequired.src/datamodel_code_generator/cli_options.py (1)
142-142: LGTM!The CLI option metadata is correctly registered under the Field Customization category, which is appropriate for an option that controls field default values.
docs/cli-reference/index.md (1)
13-13: LGTM!Documentation correctly updated to reflect the new
--default-valuesoption with the field count incremented from 22 to 23 and the option properly indexed.Also applies to: 63-63
src/datamodel_code_generator/_types/parser_config_dicts.py (1)
150-150: LGTM!The
default_value_overridesfield is correctly added toParserConfigDictwith consistent typing.src/datamodel_code_generator/config.py (1)
203-203: All verification points are satisfied.The
default_value_overridesfield is properly integrated:
- CLI argument
--default-valuesis defined in arguments.py (lines 829-841) with comprehensive help text describing JSON format and hierarchical (scoped and flat) key support- JSON file loading and parsing logic exists in main.py (lines 1196-1211) with proper error handling for JSONDecodeError, type validation, and string key validation
- File path validation is handled by the
validate_file()field validator in the Config class, which opens the file in read-text mode and validates the path exists- The field is properly wired through the codebase: passed to the generator, used in reference.py for field-level default overrides, and integrated with watch mode
src/datamodel_code_generator/arguments.py (1)
828-839:--default-valuesoption wiring matches existing CLI patternsThe new
--default-valuesargument cleanly mirrors--aliases(same group,type=Path, rich help text) and exposes the documented flat vs scoped override behavior without impacting other options. Looks good.docs/cli-reference/field-customization.md (1)
9-9: Index entry for--default-valuesis correctly wiredThe new table row correctly links to
#default-valuesand succinctly summarizes the option. No issues here.src/datamodel_code_generator/parser/base.py (1)
879-901: ModelResolver now correctly receivesdefault_value_overridesThreading
config.default_value_overridesinto the mainModelResolverinstance is consistent with how other config knobs are passed and confines overrides to the central resolver used for field/class resolution. The change is minimal and looks safe.tests/main/graphql/test_main_graphql.py (1)
788-801: LGTM! Well-structured test for default values override feature.The test appropriately exercises the interaction between
--use-defaultand--default-valuesflags in the GraphQL code generation path, following the established test patterns in this file.tests/main/test_public_api_signature_baseline.py (2)
77-77: LGTM! Backward-compatible API extension.The addition of the optional
default_value_overridesparameter maintains backward compatibility while extending the API to support the new feature.
212-212: LGTM! Consistent parameter addition.The
default_value_overridesparameter is added consistently to the Parser baseline, matching the pattern used in the generate function baseline.src/datamodel_code_generator/watch.py (2)
28-28: LGTM! Consistent parameter threading for watch mode.The addition of
default_value_overridesparameter ensures that watch mode respects default value overrides, maintaining feature parity with one-shot generation.
59-59: LGTM! Parameter correctly forwarded.The
default_value_overridesparameter is properly passed through torun_generate_from_config, ensuring the overrides are applied during regeneration on file changes.tests/main/test_main_general.py (1)
2186-2209: LGTM! Comprehensive error handling tests.These tests appropriately verify error handling for the
--default-valuesoption, covering both invalid JSON syntax and incorrect data type scenarios. The error messages are clear and actionable for users.tests/data/expected/main/jsonschema/jsonschema_default_values_override.py (1)
12-13: No changes required. The__root__: Anyusage is correct.This file represents the expected output with default Pydantic version (v1 compatibility). The codebase uses a consistent naming convention: expected output files without a version suffix generate Pydantic v1 syntax (using
__root__), while files with an explicit_pydantic_v2suffix generate Pydantic v2 syntax (usingRootModel[T]). Since there is no correspondingjsonschema_default_values_override_pydantic_v2.pyvariant for this test, the v1 output shown is the intended behavior.src/datamodel_code_generator/parser/jsonschema.py (3)
2421-2427: Good implementation of default value resolution.The integration correctly:
- Uses
original_field_name(not sanitizedfield_name) for override lookup, ensuring schema-level matching- Passes
class_nameto enable scoped overrides (ClassName.field_name)- Resolves overrides before field construction
2429-2434: Correct integration of override defaults with required field logic.The code correctly uses
effective_has_default(line 2430) instead offield.has_defaultwhen determining if a field should be required. This ensures that fields with overridden defaults are properly marked as optional whenapply_default_values_for_required_fieldsis enabled.
1064-1082: Proper propagation of resolved default values to field construction.The resolved
default_valueandhas_defaultare correctly used in:
- Field default assignment (line 1064)
- Nullable determination with strict_nullable mode (line 1072)
- Field metadata (line 1082)
This ensures the override values are consistently applied throughout field creation.
tests/data/expected/main/openapi/default_values_parameters_use_default.py (1)
16-16: No changes required. This test data file is specifically for Pydantic v1 code generation. The generator correctly usesparse_obj()for v1 output and has a separate Pydantic v2 implementation that usesmodel_validate()instead. The use ofparse_obj()in this file is appropriate for the v1 target.Likely an incorrect or invalid review comment.
tests/main/jsonschema/test_main_jsonschema.py (1)
28-41: DEFAULT_VALUES_DATA_PATH import is consistent with existing test data wiringThe added
DEFAULT_VALUES_DATA_PATHimport matches the existing pattern (e.g.,ALIASES_DATA_PATH) and keeps all test data paths centralized intests.main.conftest. No issues here.tests/main/openapi/test_main_openapi.py (2)
34-45: ImportingDEFAULT_VALUES_DATA_PATHfits existing test helpers patternBringing in
DEFAULT_VALUES_DATA_PATHnext toDATA_PATHkeeps the test data wiring consistent and avoids hard-coded paths; no issues here.
4693-4709: New OpenAPI test for--use-default+--default-valueslooks well‑wiredThe test name, docstring, input/output fixtures, and CLI arguments are consistent with the rest of the OpenAPI suite and correctly scope generation to
pathsandparameterswhile feeding the JSON defaults file. Nothing to change from a logic perspective.src/datamodel_code_generator/parser/graphql.py (1)
334-422: GraphQL default override flow inparse_fieldis coherent with the new resolver APIUsing
original_field_nameandclass_nameto feedresolve_default_value, then basingrequiredoneffective_has_defaultwhenapply_default_values_for_required_fieldsis enabled, lines up with how JsonSchema/OpenAPI now behave. Propagatingeffective_default/effective_has_defaultinto the constructed field ensures Pydantic/msgspec generation sees the resolved defaults correctly. No functional issues spotted here.Please just confirm your GraphQL default override documentation (or examples) assumes keys of the form
TypeName.fieldName(matchingobj.name), since that’s what this implementation uses for theclass_namescope.src/datamodel_code_generator/reference.py (2)
550-550: LGTM: Parameter addition follows existing patterns.The
default_value_overridesparameter is appropriately typed and positioned to maintain backward compatibility.
611-614: LGTM: Instance variable storage follows best practices.The defensive copy pattern
{**default_value_overrides}prevents external mutation, and the empty dict default is consistent with howaliasesare handled.src/datamodel_code_generator/__main__.py (5)
179-179: LGTM: Field validator correctly includes the new field.Adding
"default_values"to the validator ensures the file is validated and opened consistently with other JSON file inputs likealiases.
866-866: LGTM: Function parameter addition maintains backward compatibility.The
default_value_overridesparameter is correctly positioned at the end with an appropriate default value.
993-993: LGTM: Correctly propagates overrides to generation.The parameter is appropriately passed to the
generate()function call.
1278-1278: LGTM: Correctly passes overrides to generation.The
default_value_overridesis properly threaded through the main generation path.
1327-1329: LGTM: Watch mode correctly receives overrides.The
default_value_overridesparameter is properly passed towatch_and_regenerate, ensuring consistent behavior between one-shot generation and watch mode.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/datamodel_code_generator/reference.py (2)
611-614: Consider clarifying the comment to be more general.The comment on line 611 specifically mentions "external JSON file", but the
default_value_overridesparameter accepts mappings from any source (CLI, config files, programmatic, etc.). Consider rewording to be more generic, e.g., "Default value overrides for field defaults".🔎 Suggested improvement
- # Default value overrides from external JSON file + # Default value overrides for field defaults self.default_value_overrides: Mapping[str, Any] = ( {} if default_value_overrides is None else {**default_value_overrides} )
1203-1203: Remove unusednoqadirective.The
# noqa: FBT001comment on line 1203 is unnecessary because the FBT001 rule (boolean positional argument) is not enabled in this project's configuration.🔎 Proposed fix
- has_default: bool, # noqa: FBT001 + has_default: bool,Based on static analysis.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/data/default_values/graphql_user_defaults.jsonis excluded by!tests/data/**/*.jsonand included by none
📒 Files selected for processing (2)
src/datamodel_code_generator/reference.pytests/data/expected/main/graphql/default_values_required_use_default.py
🧰 Additional context used
🧬 Code graph analysis (2)
tests/data/expected/main/graphql/default_values_required_use_default.py (1)
src/datamodel_code_generator/model/type_alias.py (1)
TypeAlias(37-42)
src/datamodel_code_generator/reference.py (2)
src/datamodel_code_generator/parser/jsonschema.py (1)
has_default(422-424)src/datamodel_code_generator/model/base.py (2)
class_name(848-850)class_name(853-857)
🪛 Ruff (0.14.10)
src/datamodel_code_generator/reference.py
1203-1203: Unused noqa directive (non-enabled: FBT001)
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). (9)
- GitHub Check: 3.11 on macOS
- GitHub Check: py312-isort6 on Ubuntu
- GitHub Check: 3.13 on Windows
- GitHub Check: 3.14 on Windows
- GitHub Check: benchmarks
- GitHub Check: 3.10 on Windows
- GitHub Check: 3.12 on Windows
- GitHub Check: 3.11 on Windows
- GitHub Check: Analyze (python)
🔇 Additional comments (4)
tests/data/expected/main/graphql/default_values_required_use_default.py (2)
1-26: LGTM! Clean GraphQL type mappings.The file header, imports, and TypeAlias definitions follow standard conventions for generated GraphQL-to-Python mappings. The use of
from __future__ import annotationsensures compatibility with union syntax across Python versions, and the comprehensive docstrings properly document each GraphQL scalar type.
29-33: LGTM! User model correctly demonstrates default value overrides.The User model structure is appropriate for testing the new
--default-valuesfeature:
- The required
idfield (no default) contrasts properly with optional fields- The
nameandstatusfields demonstrate the default value override behavior ('default_user', 'active')- The
typename__field with__typenamealias follows standard GraphQL introspection conventionssrc/datamodel_code_generator/reference.py (2)
550-550: LGTM! Parameter addition follows best practices.The new
default_value_overridesparameter is correctly placed at the end for backward compatibility, uses an immutableMappinginterface, and follows the standard optional parameter pattern.
1199-1234: Excellent implementation! Clear hierarchical resolution logic.The
resolve_default_valuemethod is well-designed:
- Clean priority system (scoped → flat → original) that mirrors the existing alias resolution pattern
- Clear documentation explaining the override hierarchy
- Efficient early return when no overrides configured
- Consistent with the pattern used in
get_valid_field_name_and_alias(lines 304-354)The method signature and return type are appropriate for the use case described in the AI summary.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
src/datamodel_code_generator/__main__.py (4)
42-42: Remove unused# noqa: TC003(Ruff RUF100).Ruff reports this
TC003ignore as unused. Since this line is already compliant, you can safely drop the# noqa: TC003here (or enableTC003globally if you really want to keep it).
506-506: Ruff: unused# noqa: UP045ondefault_values.Ruff flags this
UP045ignore as unused. Given the surrounding fields already use the same pattern, either enableUP045in your config or drop the directive on this new line for a clean lint run.
889-899: Extendrun_generate_from_configalias type hint to allow list-valued aliases.
run_generate_from_configstill annotatesaliasesasdict[str, str] | None, but_validate_aliasesandFieldNameResolvernow supportMapping[str, str | list[str]]. At runtime this works fine, but the type hint is now misleading for callers and static type checkers.Consider widening the annotation, e.g.:
Suggested type-hint adjustment
-def run_generate_from_config( # noqa: PLR0913, PLR0917 - config: Config, - input_: Path | str | ParseResult, - output: Path | None, - extra_template_data: dict[str, Any] | None, - aliases: dict[str, str] | None, +def run_generate_from_config( # noqa: PLR0913, PLR0917 + config: Config, + input_: Path | str | ParseResult, + output: Path | None, + extra_template_data: dict[str, Any] | None, + aliases: dict[str, str | list[str]] | None,Also applies to: 1025-1026
1231-1246: Ruff: unused# noqa: T201on newRuff flags the
# noqa: T201ignores here as unused. Since these
- Enable
T201in Ruff so the directives have an effect, or- Drop the
# noqa: T201on these new lines for consistency with your current Ruff config.src/datamodel_code_generator/reference.py (2)
1199-1233: Default override resolution logic is clear; consider skipping the scoped key when class_name isNone.The scoped‑then‑flat lookup order (
ClassName.fieldthenfield) is straightforward and matches the documented behavior. Two minor nits:
- When
class_nameisNone, you still computescoped_key = "None.<field>", which is harmless but a bit odd.- If you ever expose
default_value_overridesexternally, this can surprise users who accidentally use"None.foo"keys.You could tighten this slightly:
Possible refinement
- scoped_key = f"{class_name}.{field_name}" - if scoped_key in self.default_value_overrides: - return self.default_value_overrides[scoped_key], True + if class_name is not None: + scoped_key = f"{class_name}.{field_name}" + if scoped_key in self.default_value_overrides: + return self.default_value_overrides[scoped_key], TrueFunctionally it’s already correct; this would just avoid constructing meaningless scoped keys.
1203-1203: Ruff: unused# noqa: FBT001onhas_defaultparameter.Ruff reports this
FBT001ignore as unused. Either enableFBT001globally or drop the directive here to keep lint noise down.src/datamodel_code_generator/parser/jsonschema.py (2)
2379-2445: parse_object_fields override integration looks sound; note coverage for synthesized allOf fields.The flow here:
- Resolves
effective_default, effective_has_defaultviamodel_resolver.resolve_default_valueusing the original schema field name and class name.- Uses
effective_has_defaultin the required‑flag calculation, so overrides participate in--use-defaultsemantics.- Passes the effective pair into
get_object_field, which in turn adjustsdefault,has_default, andnullable.This aligns with the intended behavior of
--default-valuesfor JSON Schema object properties.One nuance: synthesized fields created in
_parse_all_of_itemfor required‑but‑missing properties are still built viadata_model_field_type(...)directly and don’t go throughresolve_default_value. If you expect overrides to apply to those synthetic required fields as well, you may want to route them throughresolve_default_value(orget_object_field) in a follow‑up.
856-860: Ruff: unused# noqa: FBT001on_create_synthetic_enum_obj’snullableparameter.Ruff reports the
FBT001ignore onnullableas unused. As with other similar hints, you can either enableFBT001or drop the directive to keep lint output clean.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/datamodel_code_generator/__main__.pysrc/datamodel_code_generator/parser/graphql.pysrc/datamodel_code_generator/parser/jsonschema.pysrc/datamodel_code_generator/parser/openapi.pysrc/datamodel_code_generator/reference.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/datamodel_code_generator/parser/graphql.py
🧰 Additional context used
🧬 Code graph analysis (3)
src/datamodel_code_generator/parser/jsonschema.py (2)
src/datamodel_code_generator/model/base.py (5)
DataModelFieldBase(151-469)field(404-406)nullable(903-905)class_name(848-850)class_name(853-857)src/datamodel_code_generator/reference.py (1)
resolve_default_value(1199-1233)
src/datamodel_code_generator/__main__.py (2)
src/datamodel_code_generator/util.py (1)
field_validator(175-191)src/datamodel_code_generator/watch.py (1)
watch_and_regenerate(23-67)
src/datamodel_code_generator/parser/openapi.py (2)
src/datamodel_code_generator/reference.py (2)
resolve_default_value(1199-1233)reference(77-79)src/datamodel_code_generator/parser/jsonschema.py (2)
has_default(422-424)get_object_field(1031-1087)
🪛 Ruff (0.14.10)
src/datamodel_code_generator/__main__.py
42-42: Unused noqa directive (non-enabled: TC003)
Remove unused noqa directive
(RUF100)
506-506: Unused noqa directive (non-enabled: UP045)
Remove unused noqa directive
(RUF100)
1231-1231: Unused noqa directive (non-enabled: T201)
Remove unused noqa directive
(RUF100)
1238-1238: Unused noqa directive (non-enabled: T201)
Remove unused noqa directive
(RUF100)
1245-1245: Unused noqa directive (non-enabled: T201)
Remove unused noqa directive
(RUF100)
src/datamodel_code_generator/parser/openapi.py
505-505: Unused noqa directive (non-enabled: PLR0912, PLR0914)
Remove unused noqa directive
(RUF100)
src/datamodel_code_generator/reference.py
1203-1203: Unused noqa directive (non-enabled: FBT001)
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.14 on macOS
- GitHub Check: py312-isort6 on Ubuntu
- GitHub Check: 3.11 on Ubuntu
- GitHub Check: 3.10 on Ubuntu
- GitHub Check: 3.10 on Windows
- GitHub Check: 3.12 on Windows
- GitHub Check: 3.13 on Ubuntu
- GitHub Check: 3.12 on macOS
- GitHub Check: 3.14 on Windows
- GitHub Check: 3.11 on Windows
- GitHub Check: 3.13 on Windows
- GitHub Check: benchmarks
- GitHub Check: Analyze (python)
🔇 Additional comments (9)
src/datamodel_code_generator/__main__.py (4)
179-191: Configdefault_valueswiring and file validation look consistent.Including
"default_values"invalidate_fileand addingdefault_values: Optional[TextIOBase]onConfigmatches howaliases/extra_template_data/custom_formatters_kwargsare handled and keeps IO concerns in one place. No functional issues spotted.Also applies to: 506-507
857-887:_load_json_confighelper is well-factored; minor behavioral note.The helper cleanly centralizes JSON loading/validation and ensures file handles are closed. Behavior and error messages are consistent with the existing
extra_template_datahandling. Only note is thatvalidatoris assumed to be total; if it ever raises instead of returning a string, that will bubble up — which is acceptable for a CLI but worth keeping in mind.
1208-1247: JSON config validation for aliases/defaults/formatters is robust and aligned.The three validators (
_validate_aliases,_validate_string_key_dict,_validate_string_mapping) paired with_load_json_configgive clear, early CLI feedback and constrain the shapes as expected:
- aliases:
str -> str | list[str]- default_values:
str -> Any- custom_formatters_kwargs:
str -> strThe error messages are precise and consistent with the existing
extra_template_dataerror style. No logic issues spotted.
1283-1293: default_value_overrides is correctly threaded through generate and watch paths.Passing
default_value_overridesintorun_generate_from_config(both initial run and watch loop) ensures overrides apply consistently for:
- direct CLI invocations
--checkmode (through the temp output)--watchregenerationsThe pattern mirrors how
aliasesandcustom_formatters_kwargsare propagated. No issues found.Also applies to: 1337-1343
src/datamodel_code_generator/reference.py (1)
524-551: ModelResolver’s default_value_overrides storage is safe and side-effect free.Taking
default_value_overrides: Mapping[str, Any] | Noneand normalizing to a new dict ({**default_value_overrides}) avoids accidental mutation of caller-owned mappings while keeping lookup O(1). This is a good choice given how frequently resolution is called.Also applies to: 611-615
src/datamodel_code_generator/parser/jsonschema.py (1)
1031-1046: get_object_field correctly incorporates effective defaults without breaking existing callers.Using
effective_default/effective_has_defaultonly wheneffective_has_default is not Nonepreserves existing behavior for all legacy call sites, while allowing override-aware callers to:
- set both
defaultandhas_defaulton theDataModelFieldBase- influence strict‑nullable handling via the updated
has_defaultvalueThe
nullableexpression reusing the newhas_defaultis consistent with how root and array paths already behave.Also applies to: 1061-1087
src/datamodel_code_generator/parser/openapi.py (3)
538-556: LGTM! Default value override resolution is correctly implemented.The code properly resolves default values with hierarchical overrides (class.field → field) and correctly computes
effective_requiredbased on theapply_default_values_for_required_fieldsflag and presence of an effective default. The resolved values are appropriately passed toget_object_field.
585-633: LGTM! Consistent default value resolution for content-based parameters.The non-schema parameter branch correctly mirrors the pattern from the schema branch:
- Properly extracts original defaults from
object_schemawhen available- Applies the same hierarchical override resolution
- Computes
effective_requiredwith identical logic- Consistently uses effective values throughout field construction, including the nullable determination
505-505: Remove unused noqa codes.The noqa directive includes
PLR0912andPLR0914, but these rules are not enabled in your linting configuration. Keep only the enabled rules in the noqa directive.🔎 Proposed fix
- def parse_all_parameters( # noqa: PLR0912, PLR0914 + def parse_all_parameters( # noqa: PLR0914 self, name: str, parameters: list[ReferenceObject | ParameterObject],Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/datamodel_code_generator/reference.py (1)
611-614: Revise comment to be implementation-agnostic.The comment mentions "external JSON file", but this attribute accepts any
Mappingregardless of source. The comment should be more generic.🔎 Suggested revision
- # Default value overrides from external JSON file + # Default value overrides for field defaults self.default_value_overrides: Mapping[str, Any] = ( {} if default_value_overrides is None else {**default_value_overrides} )tests/main/jsonschema/test_main_jsonschema.py (2)
7461-7483: CLI doc + default-values override test are consistent with existing patternsThe new
@pytest.mark.cli_docblock andtest_main_jsonschema_default_values_overrideare wired consistently with other CLI doc tests:
options,input_schema,cli_args, andgolden_outputalign with the runtime test configuration.- The CLI docs use a relative path (
default_values/scoped_defaults.json) while the test usesDEFAULT_VALUES_DATA_PATH / "scoped_defaults.json", matching how aliases are handled elsewhere.If you want to make the docs a bit clearer, you could optionally mention how
--default-valuesinteracts with--use-default(e.g., that it can override schema defaults and/or supply defaults where none exist), but functionally this looks good.
7486-7499: allOf + --use-default coverage for default overrides looks solid
test_main_jsonschema_default_values_allofgives good coverage for the interaction between--use-defaultand--default-valuesin anallOfinheritance scenario, and the paths/expected filename are consistent with the rest of the suite.If you later expand this area, you might consider an additional case without
--use-default(pure override of existing schema defaults only), but that’s not strictly necessary for this PR.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
tests/data/default_values/allof_defaults.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/default_values/invalid_formatters_kwargs.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/jsonschema/default_values_allof.jsonis excluded by!tests/data/**/*.jsonand included by none
📒 Files selected for processing (4)
src/datamodel_code_generator/reference.pytests/data/expected/main/jsonschema/jsonschema_default_values_allof.pytests/main/jsonschema/test_main_jsonschema.pytests/main/test_main_general.py
✅ Files skipped from review due to trivial changes (1)
- tests/data/expected/main/jsonschema/jsonschema_default_values_allof.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/main/test_main_general.py
🧰 Additional context used
🧬 Code graph analysis (1)
src/datamodel_code_generator/reference.py (2)
src/datamodel_code_generator/parser/jsonschema.py (1)
has_default(422-424)src/datamodel_code_generator/model/base.py (2)
class_name(848-850)class_name(853-857)
🪛 Ruff (0.14.10)
src/datamodel_code_generator/reference.py
1203-1203: Unused noqa directive (non-enabled: FBT001)
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.14 on macOS
- GitHub Check: 3.11 on Windows
- GitHub Check: py312-isort7 on Ubuntu
- GitHub Check: 3.10 on Ubuntu
- GitHub Check: py312-pydantic1 on Ubuntu
- GitHub Check: 3.10 on Windows
- GitHub Check: py312-isort6 on Ubuntu
- GitHub Check: 3.12 on Windows
- GitHub Check: py312-black23 on Ubuntu
- GitHub Check: 3.13 on Windows
- GitHub Check: 3.14 on Windows
- GitHub Check: Analyze (python)
- GitHub Check: benchmarks
🔇 Additional comments (2)
src/datamodel_code_generator/reference.py (1)
550-550: LGTM!The new parameter is well-typed and properly positioned as an optional parameter with a None default, maintaining backward compatibility.
tests/main/jsonschema/test_main_jsonschema.py (1)
28-41: Shared DEFAULT_VALUES_DATA_PATH import looks correctImporting
DEFAULT_VALUES_DATA_PATHalongside the other shared test data paths keeps the JSON override file locations centralized and mirrors the existingALIASES_DATA_PATHpattern; no changes needed here.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/datamodel_code_generator/reference.py (1)
1199-1218: Previous issue resolved; remove unused noqa directive.The logic correctly implements hierarchical override resolution (scoped → flat → original), and the critical issue from the previous review (flat overrides being skipped when
class_name is None) has been properly addressed.However, the
noqa: FBT001directive on line 1203 is unused and should be removed.🔎 Proposed fix
def resolve_default_value( self, field_name: str, original_default: Any, - has_default: bool, # noqa: FBT001 + has_default: bool, class_name: str | None, ) -> tuple[Any, bool]:
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/datamodel_code_generator/reference.py
🧰 Additional context used
🧬 Code graph analysis (1)
src/datamodel_code_generator/reference.py (2)
src/datamodel_code_generator/parser/jsonschema.py (1)
has_default(422-424)src/datamodel_code_generator/model/base.py (2)
class_name(848-850)class_name(853-857)
🪛 Ruff (0.14.10)
src/datamodel_code_generator/reference.py
1203-1203: Unused noqa directive (non-enabled: FBT001)
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). (14)
- GitHub Check: py312-isort7 on Ubuntu
- GitHub Check: py312-black23 on Ubuntu
- GitHub Check: py312-black22 on Ubuntu
- GitHub Check: 3.10 on macOS
- GitHub Check: 3.10 on Windows
- GitHub Check: 3.13 on Windows
- GitHub Check: 3.11 on Windows
- GitHub Check: 3.10 on Ubuntu
- GitHub Check: 3.12 on Windows
- GitHub Check: 3.13 on macOS
- GitHub Check: 3.14 on Ubuntu
- GitHub Check: 3.14 on Windows
- GitHub Check: Analyze (python)
- GitHub Check: benchmarks
🔇 Additional comments (2)
src/datamodel_code_generator/reference.py (2)
550-550: LGTM! Well-designed parameter addition.The new
default_value_overridesparameter is appropriately optional with a sensible default, uses the immutableMappinginterface, and is positioned at the end to maintain backward compatibility.
611-614: LGTM! Proper defensive initialization.The attribute initialization correctly creates a defensive copy of the provided mapping to prevent external mutations, with a clean fallback to an empty dict.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/datamodel_code_generator/reference.py (1)
1199-1217: LGTM: Correct override resolution logic.The implementation now properly supports both scoped (
ClassName.field_name) and flat (field_name) overrides. The previous issue where flat overrides were skipped whenclass_name is Nonehas been successfully addressed by removing theclass_name is Nonecheck from the early return.The logic flow is clean: check for overrides → try scoped key → try flat key → fallback to original.
📝 Optional cleanup: Remove unused noqa directive
Line 1203 has an unused
noqadirective sinceFBT001is not enabled in the Ruff configuration:- has_default: bool, # noqa: FBT001 + has_default: bool,
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/datamodel_code_generator/reference.py
🧰 Additional context used
🧬 Code graph analysis (1)
src/datamodel_code_generator/reference.py (2)
src/datamodel_code_generator/parser/jsonschema.py (1)
has_default(422-424)src/datamodel_code_generator/model/base.py (2)
class_name(848-850)class_name(853-857)
🪛 Ruff (0.14.10)
src/datamodel_code_generator/reference.py
1203-1203: Unused noqa directive (non-enabled: FBT001)
Remove unused noqa directive
(RUF100)
🔇 Additional comments (2)
src/datamodel_code_generator/reference.py (2)
550-550: LGTM: Clean parameter addition.The parameter type
Mapping[str, Any] | Nonecorrectly uses the read-onlyMappinginterface, allowing callers flexibility while preventing unintended mutations.
611-614: LGTM: Proper defensive initialization.Creating a defensive copy with
{**default_value_overrides}prevents external mutation, and storing asMapping[str, Any]clearly signals the immutable interface.
Breaking Change AnalysisResult: No breaking changes detected Reasoning: This PR adds a new This analysis was performed by Claude Code Action |
|
🎉 Released in 0.52.0 This PR is now available in the latest release. See the release notes for details. |
1 participant
Fixes: #2457
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.