Replace Rawdict class with dict annotations#21
Merged
Conversation
- Remove RawDict class from models.py - Replace all RawDict type hints with dict - Replace RawDict() instantiations with direct dict access using .get() - Update test_models.py to treat dict as primitive type - Simplify check_model to accept plain dicts for unstructured data This simplifies the codebase by removing a wrapper class that didn't add functionality beyond being a type marker. The test coverage for detecting missing model classes is intentionally removed in favor of simpler, more Pythonic code using plain dicts. All 188 tests pass.
Replace plain dict type hints with Annotated[dict, "unstructured"] to mark intentionally unstructured data while maintaining test coverage. Changes: - Add Annotated imports to models.py and events.py - Annotate all unstructured dict fields: changes, config, error, _links, reactions, client_payload, head, base, payload (in Deployment) - Update test_models.py to detect and validate Annotated types: - Add _is_unstructured_dict() helper to check for annotations - Update check_model() to enforce annotation requirement - Update _is_primitive_type() to treat annotated dicts as primitives - Update _validate_simple_type() to handle Annotated and Optional[Annotated] - Add include_extras=True to get_type_hints() calls - Add missing _links type hint to Comment class Benefits: - Zero runtime overhead (plain dict at runtime) - Tests can detect unannotated dicts and enforce model classes - Self-documenting type hints - Restores test coverage lost when RawDict was removed All 188 tests pass.
Critical fixes: - Fix PullRequestReviewEvent.changes: was assigning pull_request instead of changes - Fix ReleaseEvent.changes: was assigning release instead of changes - Fix type hints: changes field should be Optional in both events Code improvements: - Extract _unwrap_annotated() helper to deduplicate Optional/Annotated unwrapping logic - Simplify _validate_simple_type() using the new helper Documentation: - Add comprehensive module docstring to models.py explaining annotation pattern - Document when to use Annotated[dict, "unstructured"] vs model classes Testing: - Add test_unannotated_dict_enforcement() to verify annotation requirement - Ensures plain dicts without annotation raise AttributeError All 189 tests pass (1 new test added).
doodla
deleted the
claude/investigate-rawdict-usage-011CUwTT3tXzyqLfnftc2Fqo
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.