Skip to content

[data] support non-string enum values in the default tool format - #10891

Open
cyx5445 wants to merge 1 commit into
hiyouga:mainfrom
cyx5445:fix/default-tool-nonstring-enums
Open

cyx5445 wants to merge 1 commit into
hiyouga:mainfrom
cyx5445:fix/default-tool-nonstring-enums

Conversation

@cyx5445

@cyx5445 cyx5445 commented Oct 14, 2026

Copy link
Copy Markdown

Preserve string enum display and serialize other values as JSON. Add regression coverage for flat and OpenAI-wrapped tools.

Preserve string enum display and serialize other values as JSON.
Add regression coverage for flat and OpenAI-wrapped tools.

Assisted-by: OpenAI Codex

@ErenAta16 ErenAta16 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.

Checked by loading tool_utils.py from main and from this head and calling the default tool_formatter on both (I could not run the pytest module here because the test package imports trl).

String enums are byte-identical to main for ["small", "large"], ["a, b", "c"], ["默认", "x"], ["1", "2"] and the empty list, both flat and OpenAI-wrapped, so existing prompts do not change. For non-string enums main raises TypeError: sequence item 0: expected str instance on [1, 2, 3], [0.5, 1.5], [True, False], [None] and on dict or list members; this head renders 1, 2, 3, 0.5, 1.5, true, false, null and {"a": 1}, [1, 2]. Flat and wrapped tools render the same. The enum join in tool_formatter is the only place in this file that formats enum values, so no sibling site needs the same change.

Nothing blocking. One optional observation: a mixed enum such as ["1", 1] renders as [1, 1], so the prompt cannot tell the string "1" from the integer 1. Leaving strings unquoted is what keeps existing prompts unchanged, so I would keep it; a sentence in the docstring or the PR description would be enough.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants