Skip to content

Add ChatRole enum for message history role validation - #518

Merged
vishal-bala merged 5 commits into
redis:mainfrom
nabilasiregar:feat/484-Add_ChatRole_enum
Mar 2, 2026
Merged

vishal-bala merged 5 commits into
redis:mainfrom
nabilasiregar:feat/484-Add_ChatRole_enum

Conversation

@nabilasiregar

@nabilasiregar nabilasiregar commented Feb 27, 2026 •

Copy link
Copy Markdown
Contributor

Changes for #484

  • Add ChatRole(str, Enum) to schema.py as the single source of truth
    for valid roles
  • Update ChatMessage.role to use ChatRole with a field_validator
    that coerces valid strings automatically
  • Align _validate_roles in base_history.py to derive valid roles from
    ChatRole instead of hardcoded values
  • Export ChatRole from the message_history package __init__.py
  • Add unit tests for valid string coercion, direct enum input, deprecated role warning message, invalid role rejection, and string serialization via to_dict()

Note

Medium Risk
Changes the public ChatMessage.role type/validation and role filtering behavior, which may affect downstream callers that assume plain strings (especially the legacy llm role), though deprecated values are still accepted with warnings.

Overview
Adds a ChatRole enum (user/assistant/system/tool) as the single source of truth for message roles, and updates ChatMessage.role to coerce valid role strings into the enum while emitting DeprecationWarnings for legacy/unknown string roles.

Aligns BaseMessageHistory._validate_roles to derive allowed roles from ChatRole (warning-but-allow for llm), exports ChatRole from message_history.__init__, and expands unit tests to cover coercion, deprecation warnings, invalid role rejection, and to_dict() role serialization.

Written by Cursor Bugbot for commit 94211c8. This will update automatically on new commits. Configure here.

@nabilasiregar

Copy link
Copy Markdown
Contributor Author

Current implementation will break existing string-based behavior (e.g. llm), will fix this asap, don't review just yet

@tylerhutcherson

Copy link
Copy Markdown
Collaborator

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

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

@vishal-bala

Copy link
Copy Markdown
Collaborator

Hey @nabilasiregar, let us know when this PR is ready for us to take a look at again 👍

@nabilasiregar

Copy link
Copy Markdown
Contributor Author

Hey @vishal-bala it's ready

@vishal-bala
vishal-bala self-requested a review March 2, 2026 12:03
@vishal-bala vishal-bala added the auto:minor Increment the minor version when merged label Mar 2, 2026

@vishal-bala vishal-bala left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

if role not in valid_roles:
raise ValueError(
f"Invalid role '{role}'. Valid roles are: {valid_roles}"
if role in valid_roles:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a slight convenience thing, but you can also just do membership checks on the enum 😄

if role in ChatRole:
    ...

@vishal-bala
vishal-bala merged commit 23ecc77 into redis:main Mar 2, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto:minor Increment the minor version when merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants