Skip to content

minor: 'role' must be system/assistant/tool, but case insensitive for now #7499

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 25, 2025
Merged

minor: 'role' must be system/assistant/tool, but case insensitive for now #7499

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dcfe543
minor: 'role' must be system/assistant/tool (case insensitive) for now
minleminzui Jun 24, 2025
399b0d9
Update protocol.py
minleminzui Jun 24, 2025
3ee6ad7
Merge branch 'main' into case_in
sleepcoo Jun 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions python/sglang/srt/entrypoints/openai/protocol.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,18 @@ class ChatCompletionMessageGenericParam(BaseModel):
reasoning_content: Optional[str] = None
tool_calls: Optional[List[ToolCall]] = Field(default=None, examples=[None])

@field_validator("role", mode="before")
@classmethod
def _normalize_role(cls, v):
Copy link
Collaborator

@CatherineSue CatherineSue Jun 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Can we add a NOTE here that this change is due to the compatibility with Spring AI?

This will help with documentaiton and removal in the future when Spring AI supports lower-cased role.

Copy link
Collaborator

@CatherineSue CatherineSue Jun 25, 2025

Choose a reason for hiding this comment

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

cc @minleminzui The PR is merged. Is it possible to add a note in a new PR?

Or could you include more details on the context? is this change currently only useful for SpringAI?

Appreciate it.

Copy link
Collaborator Author

@minleminzui minleminzui Jun 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

cc @minleminzui The PR is merged. Is it possible to add a note in a new PR?

Or could you include more details on the context? is this change currently only useful for SpringAI?

Appreciate it.

sure， I'll provide it later.

Copy link
Collaborator

@CatherineSue CatherineSue Jul 15, 2025

Choose a reason for hiding this comment

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

@minleminzui friendly ping :)

Copy link
Collaborator

@sleepcoo sleepcoo Jul 16, 2025

Choose a reason for hiding this comment

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

@minleminzui friendly ping :)

I confirmed that under normal circumstances, Spring outputs lowercase, but some ways of writing Spring can be bypassed to become uppercase.

if isinstance(v, str):
v_lower = v.lower()
if v_lower not in {"system", "assistant", "tool"}:
raise ValueError(
"'role' must be one of 'system', 'assistant', or 'tool' (case-insensitive)."
)
return v_lower
raise ValueError("'role' must be a string")


class ChatCompletionMessageUserParam(BaseModel):
role: Literal["user"]
Expand Down
Loading