fix(sglang): review follow-ups for effort_budgets

- warn on an effort_budgets entry without a token count (high=)
- update comments: enable_thinking=false still wins over a listed level
- document that precedence in text-generation.md
- cover xhigh/max and the empty-entry warning in the tests

Signed-off-by: pos-ei-don <1822533+pos-ei-don@users.noreply.github.com>
(cherry picked from commit 0bd723a652)
This commit is contained in:
pos-ei-don authored and Ettore Di Giacinto committed 2026-10-25 08:50:08 +00:00
1 parent 073adc50c9
commit 1c125a8897
3 files changed
+30 -13

No files matched your search

+11 -7
View File
@@ -186,6 +186,9 @@ class BackendServicer(backend_pb2_grpc.BackendServicer):
if not sep or not level:
print(f"effort_budgets entry {entry!r} is not level=tokens, ignoring it", file=sys.stderr)
continue
if not raw.strip():
print(f"effort_budgets entry {entry!r} has no token count, ignoring it", file=sys.stderr)
continue
tokens = cls._parse_thinking_budget(raw)
if tokens is not None:
budgets[level] = tokens
@@ -349,8 +352,7 @@ class BackendServicer(backend_pb2_grpc.BackendServicer):
# read it: every effort level got the same fixed `thinking_budget`, so
# low/medium/high produced identical reasoning. With this map a request
# gets the budget of its effort level; levels absent from the map, and
# requests without an effort, keep `thinking_budget`. Same semantics as
# vLLM's proposed `--reasoning-effort-budgets` (vllm-project/vllm#55432).
# requests without an effort, keep `thinking_budget`.
self.effort_budgets: dict = self._parse_effort_budgets(opts.get("effort_budgets"))
# Model-level default for whether the chat template opens a reasoning
@@ -525,7 +527,8 @@ class BackendServicer(backend_pb2_grpc.BackendServicer):
def _thinking_default(self, request) -> Optional[bool]:
"""Whether this request should render with reasoning on, off, or unset.
Per-request ``Metadata["enable_thinking"]`` wins; the model-level
Per-request ``Metadata["enable_thinking"]`` wins; next, an effort level
listed in ``effort_budgets`` turns reasoning on; the model-level
``reasoning_default`` option fills in when the request is silent.
Returns None when neither says anything, leaving template behaviour
untouched.
@@ -534,10 +537,11 @@ class BackendServicer(backend_pb2_grpc.BackendServicer):
if wanted in ("true", "false"):
return wanted == "true"
# An effort level listed in `effort_budgets` is an explicit request to
# think. The Go side only sets enable_thinking for the levels it knows
# (minimal/low/medium/high); for others such as `xhigh` or `max` it sets
# nothing, so `reasoning_default: off` would win and asking for MORE
# reasoning would yield NONE.
# think. Normally the Go side already sends enable_thinking for known
# levels; this covers levels it does not set it for (custom names in
# the map, or callers that only send reasoning_effort), where
# `reasoning_default: off` would otherwise win. An explicit
# enable_thinking=false above still takes precedence.
if self.effort_budgets and self._effort(request) in self.effort_budgets:
return True
if self.reasoning_default == "off":
+18 -5
View File
@@ -361,24 +361,37 @@ class TestSglangHelpers(unittest.TestCase):
self.assertEqual(servicer._parse_effort_budgets("low=abc;medium;=5;high=0;xhigh=100"), {"xhigh": 100})
self.assertEqual(servicer._parse_effort_budgets(None), {})
self.assertEqual(servicer._parse_effort_budgets(""), {})
self.assertEqual(servicer._parse_effort_budgets(";;low=512;"), {"low": 512})
def test_effort_budgets_empty_value_warns(self):
"""`high=` is skipped like any other invalid entry, and says so."""
import contextlib
import io
servicer = self._servicer()
err = io.StringIO()
with contextlib.redirect_stderr(err):
self.assertEqual(servicer._parse_effort_budgets("high=;max=16384"), {"max": 16384})
self.assertIn("'high='", err.getvalue())
def test_effort_level_selects_its_budget(self):
"""The request's reasoning_effort (Metadata, forwarded by the Go side)
picks its own budget, so low/medium/high stop being identical."""
servicer = self._servicer()
servicer.thinking_budget = 256
servicer.effort_budgets = {"low": 512, "medium": 2048, "high": 8192}
for level, budget in (("low", 512), ("medium", 2048), ("HIGH", 8192)):
servicer.effort_budgets = {"low": 512, "medium": 2048, "high": 8192, "xhigh": 16384, "max": 32768}
for level, budget in (("low", 512), ("medium", 2048), ("HIGH", 8192), ("xhigh", 16384), ("max", 32768)):
params = servicer._build_sampling_params(self._sampling_request({"reasoning_effort": level}))
self.assertEqual(params["custom_params"], {"thinking_budget": budget}, level)
def test_listed_effort_turns_thinking_on_despite_default_off(self):
"""`xhigh`/`max` get no enable_thinking from the Go side; if the level is
in effort_budgets it must still think instead of falling to default off."""
"""A request that carries only reasoning_effort (no enable_thinking) for a
level listed in effort_budgets must think instead of falling to default off."""
servicer = self._servicer()
servicer.reasoning_default = "off"
servicer.effort_budgets = {"xhigh": 16384}
servicer.effort_budgets = {"xhigh": 16384, "max": 16384}
self.assertTrue(servicer._thinking_default(_request(metadata={"reasoning_effort": "xhigh"})))
self.assertTrue(servicer._thinking_default(_request(metadata={"reasoning_effort": "max"})))
self.assertFalse(servicer._thinking_default(_request(metadata={"reasoning_effort": "ultra"})))
self.assertFalse(servicer._thinking_default(
_request(metadata={"reasoning_effort": "xhigh", "enable_thinking": "false"})))