fix(types): get_enum_from_literal/get_enum_from_choice drop values that collide under str()#1926
Conversation
…at collide under str()
Both functions built their Enum's name->value mapping with
`{str(x): x for x in values}`. Since str(1) == str("1") == "1",
Literal["1", 1] or Choice(["1", 1]) silently collapsed to a single
enum member -- whichever value came last in iteration order won, the
other was overwritten in the dict comprehension before Enum ever saw
it. Reachable in production via models/gemini.py's structured-output
enum construction (Gemini text/x.enum response schemas) for any
Literal/Choice mixing a string and a numeric value that happen to
stringify the same way.
Fix: only disambiguate when a genuine str()-collision is detected,
prefixing the colliding values' keys with their type name
(e.g. "str_1" vs "int_1"). Non-colliding values keep their original
str()-based member name unchanged, preserving existing behavior and
the existing test's attribute-access assertions
(getattr(complex_enum, "1"), "True", "None", etc.).
Added two new tests covering the str()-collision case for both
functions, verifying both values survive as distinct members. TDD
red->green verified: reverting only utils.py reproduces
`{1} == {1, '1'}` (the string value silently dropped); reapplying
passes. Full tests/types/ suite: 234 passed. ruff/mypy clean via
pre-commit.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Traced the collision mechanism: |
| else: | ||
| for value in group: | ||
| result[f"{type(value).__name__}_{value}"] = value | ||
| return result |
There was a problem hiding this comment.
This fallback key can still collide when two values have the same type name and string form. For example, Choice accepts two enum members from classes both named State with a member named READY; both become State_State.READY, so one is still dropped. Could the disambiguator use an index rather than another value-derived key?
|
📚 Documentation preview: https://dottxt-ai.github.io/outlines/pr-preview/pr-1926/ Preview updates automatically with each commit. |
Problem
Both
get_enum_from_literalandget_enum_from_choicebuilt theirEnum's name→value mapping with{str(x): x for x in values}. Sincestr(1) == str("1") == "1",Literal["1", 1]orChoice(["1", 1])silently collapsed to a single enum member — whichever value came last in iteration order won, and the other was overwritten in the dict comprehension beforeEnumever saw it.Reachable in production via
models/gemini.py's structured-output enum construction (Geminitext/x.enumresponse schemas) for anyLiteral/Choicemixing a string and a numeric value that happen to stringify the same way.Fix
Only disambiguate when a genuine
str()-collision is detected, prefixing the colliding values' keys with their type name (e.g."str_1"vs"int_1"). Non-colliding values keep their originalstr()-based member name unchanged, preserving existing behavior and the existing test's attribute-access assertions (getattr(complex_enum, "1"),"True","None", etc.) — this was checked and deliberately preserved rather than switching to an always-prefixed scheme, since that would've silently changed the public member-naming contract for the (much more common) non-colliding case.Testing
str()-collision case for both functions, verifying both values survive as distinct members.utils.pyreproduces{1} == {1, '1'}(the string value silently dropped, only the int survives); reapplying passes.tests/types/suite: 234 passed.ruffandmypyclean viapre-commit run.types/utils.py.AI-Generated disclosure
Found via an AI-assisted code review pass (Claude Code) over
outlines/src/outlines/types/. I personally reproduced the value-dropping collision, checked the existing test's attribute-name assertions to make sure my fix wouldn't silently break the non-colliding naming contract, verified the fix, and ran the relevant test suite plus lint/type checks before submitting.