Extract shared from_dict validation helpers for model classes (#70)#80
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughIntroduces models/from_dict_validation.py with reusable require_* helpers and updates Workspace, Composer, WorkspaceLocalComposer, Bubble, CliSessionMeta, and ExportEntry to use those helpers for from_dict validation instead of inline isinstance checks and manual SchemaError raising. ChangesValidation Helpers and Model Migrations
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@models/from_dict_validation.py`:
- Around line 38-44: The code currently uses value = raw.get(key) which converts
absent keys to None and causes the isinstance check to raise an "invalid field"
SchemaError; change the logic to first detect missing keys (e.g., if key not in
raw) and raise the appropriate "missing required field" SchemaError for that
key/model before performing type/emptiness checks on value, then keep the
existing isinstance(value, str) and non-empty validation to raise the "invalid
field" SchemaError when the key is present but wrong; apply the same fix to the
other helper at the corresponding block that currently uses raw.get(...) (the
block around lines 56-62).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2c2ff1c2-3af3-4383-81aa-6c6507cb389d
📒 Files selected for processing (5)
models/cli_session.pymodels/conversation.pymodels/export.pymodels/from_dict_validation.pymodels/workspace.py
…ing-field helpers.
|
models/from_dict_validation.py — add a small tests/test_from_dict_validation.py asserting message text for require_non_empty_str_fields / require_non_empty_str_field: absent key → missing required field, wrong type → invalid field. test_schema_error_message_distinguishes_missing_from_invalid covers the contract generically but not these helpers post-f846aaf. models/from_dict_validation.py:69 — expand require_truthy docstring to note falsy values (None, "", 0) are treated as missing (matches prior CliSessionMeta behavior). |
Summary
Centralizes the repeated
from_dictvalidation pattern (dict check, required keys, type checks,SchemaErrorraising) in a newmodels/from_dict_validation.pymodule. All six model classes that used copy-pasted boilerplate now call shared helpers; model-specific rules stay in each class.require_dict,require_key,require_non_empty_str,require_non_empty_str_field,require_non_empty_str_fields,require_truthy,require_type,require_optional_strWorkspace,Composer,WorkspaceLocalComposer,Bubble,CliSessionMeta,ExportEntrySchemaErrormessage shape (missing vs invalid field, field names, type hints); frozen dataclass constructors; service-layer call sitesCloses #70
Test plan
pytest tests/test_models.py— all model schema tests pass unchangedpytest tests/test_models_wired_at_read_sites.py— read-site wiring still callsfrom_dictpytest(306 passed, 4 skipped)Summary by CodeRabbit