[Fix] Treat an empty grammar constraint as unset in SamplingParams (#33328)
This commit is contained in:
@@ -147,7 +147,7 @@ class GrammarManager:
|
||||
key = ("regex", req.sampling_params.regex)
|
||||
elif req.sampling_params.ebnf is not None:
|
||||
key = ("ebnf", req.sampling_params.ebnf)
|
||||
elif req.sampling_params.structural_tag:
|
||||
elif req.sampling_params.structural_tag is not None:
|
||||
key = ("structural_tag", req.sampling_params.structural_tag)
|
||||
|
||||
value, cache_hit = self.grammar_backend.get_cached_or_future_value(
|
||||
|
||||
@@ -134,6 +134,12 @@ class SamplingParams(msgspec.Struct, kw_only=True, array_like=True):
|
||||
self.no_stop_trim if self.no_stop_trim is not None else False
|
||||
)
|
||||
|
||||
# An empty grammar constraint means "unset", not "constrain to nothing".
|
||||
self.json_schema = self.json_schema or None
|
||||
self.regex = self.regex or None
|
||||
self.ebnf = self.ebnf or None
|
||||
self.structural_tag = self.structural_tag or None
|
||||
|
||||
# Process some special cases
|
||||
if 0 <= self.temperature < _SAMPLING_EPS:
|
||||
# top_k = 1 means greedy sampling
|
||||
|
||||
@@ -189,6 +189,21 @@ class TestProcessReqWithGrammar(unittest.TestCase):
|
||||
("structural_tag", '{"structures": [], "triggers": []}'),
|
||||
)
|
||||
|
||||
def test_falsy_structural_tag_still_resolves_a_key(self):
|
||||
"""The selection chain must cover every value the entry condition admits.
|
||||
A falsy-but-set constraint used to match no branch and hit the key lookup
|
||||
with nothing assigned.
|
||||
"""
|
||||
mgr = self._make_mgr()
|
||||
future = Future()
|
||||
mgr.grammar_backend.get_cached_or_future_value.return_value = (future, False)
|
||||
|
||||
req = _make_req(structural_tag="")
|
||||
result = mgr.process_req_with_grammar(req)
|
||||
|
||||
self.assertTrue(result)
|
||||
self.assertEqual(req.grammar_key, ("structural_tag", ""))
|
||||
|
||||
def test_cache_hit_returns_false(self):
|
||||
"""Cache hit should NOT add to grammar queue."""
|
||||
mgr = self._make_mgr()
|
||||
|
||||
@@ -73,6 +73,16 @@ class TestSamplingParamsInit(CustomTestCase):
|
||||
sp = SamplingParams(stop_token_ids=[])
|
||||
self.assertIsNone(sp.stop_token_ids)
|
||||
|
||||
def test_empty_grammar_constraint_becomes_none(self):
|
||||
"""An empty grammar string means "unset", not "constrain to nothing".
|
||||
Left as "" it reads as set to the is-not-None checks downstream while
|
||||
the constraint selection skips it.
|
||||
"""
|
||||
for field in ("json_schema", "regex", "ebnf", "structural_tag"):
|
||||
with self.subTest(field=field):
|
||||
sp = SamplingParams(**{field: ""})
|
||||
self.assertIsNone(getattr(sp, field))
|
||||
|
||||
|
||||
class TestSamplingParamsVerify(CustomTestCase):
|
||||
|
||||
|
||||
Reference in New Issue
Block a user