Add ChatRole enum for message history role validation#518
Add ChatRole enum for message history role validation#518nabilasiregar wants to merge 5 commits intoredis:mainfrom
Conversation
|
Current implementation will break existing string-based behavior (e.g. llm), will fix this asap, don't review just yet |
All good - we can hold |
| DeprecationWarning, | ||
| stacklevel=2, | ||
| ) | ||
| return v |
There was a problem hiding this comment.
Validator accepts any string as deprecated instead of rejecting invalid
High Severity
The coerce_role validator treats every non-ChatRole string as "deprecated," emitting a DeprecationWarning and accepting it. Truly invalid values like "potato" or "" are never rejected with a ValidationError. This contradicts the PR's stated goal and is inconsistent with _validate_roles in base_history.py, which correctly maintains a separate deprecated_roles set and raises ValueError for unknown strings. The validator needs to distinguish genuinely deprecated roles (e.g. "llm") from invalid ones.
Additional Locations (1)
There was a problem hiding this comment.
_validate_roles in base_history.py previously enforced valid_roles = {"system", "user", "llm", "tool"} for filtering (get_recent(role=...)), and "llm" was the only role removed from that set.
But ChatMessage.role in schema.py was previously role: str with no validation (any string was accepted at message creation). To stay non-breaking, coerce_role warns on all deprecated strings rather than raising. Raising ValueError on strings like "admin" or "bot" would break existing users who stored messages with custom roles, which the old schema never prevented.


Changes for #484
ChatRole(str, Enum)toschema.pyas the single source of truthfor valid roles
ChatMessage.roleto useChatRolewith afield_validatorthat coerces valid strings automatically
_validate_rolesinbase_history.pyto derive valid roles fromChatRoleinstead of hardcoded valuesChatRolefrom themessage_historypackage__init__.pyto_dict()Note
Medium Risk
Changes the public
ChatMessage.roletype/validation and role filtering behavior, which may affect downstream callers that assume plain strings (especially the legacyllmrole), though deprecated values are still accepted with warnings.Overview
Adds a
ChatRoleenum (user/assistant/system/tool) as the single source of truth for message roles, and updatesChatMessage.roleto coerce valid role strings into the enum while emittingDeprecationWarnings for legacy/unknown string roles.Aligns
BaseMessageHistory._validate_rolesto derive allowed roles fromChatRole(warning-but-allow forllm), exportsChatRolefrommessage_history.__init__, and expands unit tests to cover coercion, deprecation warnings, invalid role rejection, andto_dict()role serialization.Written by Cursor Bugbot for commit 94211c8. This will update automatically on new commits. Configure here.