fix(context): reject non-dict models/views in MDL conversion#2567
fix(context): reject non-dict models/views in MDL conversion#2567Bartok9 wants to merge 1 commit into
Conversation
convert_mdl_to_project assumed every models/views entry was a dict and crashed in _convert_keys_to_snake / key checks. Raise a clear ValueError. validate_project now records ValidationError for non-dict load entries.
WalkthroughThe change adds dictionary-shape checks for model and view entries during MDL conversion and project validation. Invalid conversion inputs raise indexed ChangesProject input validation
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
core/wren/tests/unit/test_convert_mdl.py (1)
388-395: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStrengthen coverage for the complete error contract.
The model test does not verify the index or actual type, and the view test does not verify the actual type. Also add tests for
validate_projectto confirm invalid entries produceValidationErrors atmodels[i]andviews[i]instead of raising.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/wren/tests/unit/test_convert_mdl.py` around lines 388 - 395, Strengthen the tests around convert_mdl_to_project so invalid model and view entries assert the complete ValueError message, including the entry index and actual type. Add validate_project tests covering invalid entries in both models and views, asserting ValidationError is returned or collected at paths models[i] and views[i] rather than raised.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@core/wren/tests/unit/test_convert_mdl.py`:
- Around line 388-395: Strengthen the tests around convert_mdl_to_project so
invalid model and view entries assert the complete ValueError message, including
the entry index and actual type. Add validate_project tests covering invalid
entries in both models and views, asserting ValidationError is returned or
collected at paths models[i] and views[i] rather than raised.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b686601a-2397-4d93-bec3-8c644d2329ae
📒 Files selected for processing (2)
core/wren/src/wren/context.pycore/wren/tests/unit/test_convert_mdl.py
Summary
convert_mdl_to_projectassumed everymodels/viewsentry was a dict; list litter crashed during snake_case convert.ValueErrornaming the index and actual type.validate_projectrecords aValidationErrorfor non-dict loaded entries instead ofAttributeError.License
core/wren/**Apache-2.0.Test plan
pytest tests/unit/test_convert_mdl.py -k non_dict -v(2 passed)Real behavior proof
Non-dict model raised TypeError-ish path inside convert; now ValueError with index.
Summary by CodeRabbit
Bug Fixes
Tests