mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-12 23:12:23 -06:00
fix(model-turn): no caller-default effort rung — local vocabularies make any code effort token unsafe
Follow-up ruling on the round-2 batch: default_reasoning_effort is removed entirely. On local lanes effort_passthrough forwards the value VERBATIM with the template as the sole authority on validity, and we explicitly do not define effort vocabularies (or floors) for local models — so a code-chosen "low" is an unvetted token, and on manual-thinking boxes it flips enable_thinking on for lanes the operator never configured, diverging from the main loop's unset. The effort scheme is now exactly the temperature scheme: explicit relay > alias > stored config > model definition > omit. Utility/guard consequences handled the honest way instead: - title gen: _TITLE_MAX_TOKENS 2048 -> 8192 (the budget must fit a full thinking pass at the MODEL'S OWN default now that code never bounds it) and the prompt enforces a hard 3-word maximum so the visible answer is trivially cheap regardless of what thinking spent. - output guard: keeps its 512 cap; an unbounded thinking model that overruns it parses to a labelled llm_error verdict (heuristic tier stands) and the documented remediation is an effort value on the guard's model alias.
This commit is contained in:
+13
-20
@@ -468,46 +468,39 @@ def test_resolve_lane_reasoning_effort_operator_rungs() -> None:
|
||||
|
||||
|
||||
def test_model_turn_effort_lower_rungs() -> None:
|
||||
# Below the lane's operator rungs, model_turn applies: caller
|
||||
# request-shaped default → in-code model definition (caps) → None
|
||||
# (wire omission — no hidden "medium" constant anywhere).
|
||||
# Below the lane's operator rungs, model_turn applies exactly one more
|
||||
# rung — the in-code model definition (caps) — then None (wire
|
||||
# omission). There is deliberately NO caller-default rung: a
|
||||
# code-chosen effort is an unvetted token on local vocabularies
|
||||
# (effort_passthrough forwards verbatim) and flips template thinking
|
||||
# toggles the operator never engaged.
|
||||
# Bare hand-built lane: nothing anywhere → the provider receives None.
|
||||
provider = _FakeProvider([CompletionResult(content="")])
|
||||
model_turn(_lane(provider), [Turn.user("x")])
|
||||
assert provider.calls[0]["reasoning_effort"] is None
|
||||
|
||||
# In-code model definition rung: a declared caps default applies.
|
||||
# In-code model definition rung: a declared caps default applies…
|
||||
caps = ModelCapabilities(default_reasoning_effort="high")
|
||||
provider2 = _FakeProvider([CompletionResult(content="")])
|
||||
model_turn(_lane(provider2, capabilities=caps), [Turn.user("x")])
|
||||
assert provider2.calls[0]["reasoning_effort"] == "high"
|
||||
|
||||
# A caller request-shaped default (budget-coupled lanes: utility,
|
||||
# output guard) beats the model-generic caps default…
|
||||
# …loses to an operator value on the lane…
|
||||
provider3 = _FakeProvider([CompletionResult(content="")])
|
||||
model_turn(
|
||||
_lane(provider3, capabilities=caps), [Turn.user("x")], default_reasoning_effort="low"
|
||||
)
|
||||
assert provider3.calls[0]["reasoning_effort"] == "low"
|
||||
|
||||
# …loses to an operator value on the lane…
|
||||
provider4 = _FakeProvider([CompletionResult(content="")])
|
||||
model_turn(
|
||||
_lane(provider4, capabilities=caps, reasoning_effort="xhigh"),
|
||||
_lane(provider3, capabilities=caps, reasoning_effort="xhigh"),
|
||||
[Turn.user("x")],
|
||||
default_reasoning_effort="low",
|
||||
)
|
||||
assert provider4.calls[0]["reasoning_effort"] == "xhigh"
|
||||
assert provider3.calls[0]["reasoning_effort"] == "xhigh"
|
||||
|
||||
# …and to an explicit relay (the "none" knob stays distinct from unset).
|
||||
provider5 = _FakeProvider([CompletionResult(content="")])
|
||||
provider4 = _FakeProvider([CompletionResult(content="")])
|
||||
model_turn(
|
||||
_lane(provider5, capabilities=caps),
|
||||
_lane(provider4, capabilities=caps),
|
||||
[Turn.user("x")],
|
||||
reasoning_effort="none",
|
||||
default_reasoning_effort="low",
|
||||
)
|
||||
assert provider5.calls[0]["reasoning_effort"] == "none"
|
||||
assert provider4.calls[0]["reasoning_effort"] == "none"
|
||||
|
||||
|
||||
def test_model_turn_fetches_config_once_per_call() -> None:
|
||||
|
||||
@@ -615,7 +615,6 @@ def model_turn(
|
||||
max_tokens: int = 4096,
|
||||
temperature: float | None = None,
|
||||
reasoning_effort: str | None = None,
|
||||
default_reasoning_effort: str | None = None,
|
||||
mint: Callable[[str], str] | None = None,
|
||||
wire_id_map: dict[str, str] | None = None,
|
||||
resolve_attachments: Callable[[list[str]], dict[str, Any]] | None = None,
|
||||
@@ -634,13 +633,14 @@ def model_turn(
|
||||
|
||||
*temperature* / *reasoning_effort* ``None`` (the defaults) inherit the
|
||||
lane's operator-resolved knobs (per-model config → stored global
|
||||
setting). When no operator spoke, temperature is OMITTED from the
|
||||
wire (the inference engine's default rules); effort falls through
|
||||
*default_reasoning_effort* — a request-shaped default for lanes whose
|
||||
token budget constrains thinking (title gen, the output guard), which
|
||||
any operator value beats — then the in-code model definition
|
||||
(``caps.default_reasoning_effort``), then omission. House rule: never
|
||||
pin either knob in code; pass an explicit value only to relay an
|
||||
setting). When no operator spoke, effort falls through the in-code
|
||||
model definition (``caps.default_reasoning_effort``), then both knobs
|
||||
are OMITTED from the wire — the inference engine's default rules.
|
||||
House rule: never pin either knob in code — there is deliberately no
|
||||
caller-supplied default rung, because a code-chosen effort value is
|
||||
an unvetted token on local vocabularies (``effort_passthrough``
|
||||
forwards it verbatim) and flips template thinking toggles the
|
||||
operator never engaged. Pass an explicit value only to relay an
|
||||
operator- or user-resolved knob (the session's own knobs, a CLI flag).
|
||||
|
||||
*resolve_attachments* materializes by-reference ``AttachmentRef``
|
||||
@@ -681,16 +681,14 @@ def model_turn(
|
||||
)
|
||||
wire = maybe_attach_vllm_chat_reasoning(wire, lane.provider, lane.registry, lane.alias, cfg=cfg)
|
||||
# The effort assignment scheme's lower rungs: explicit relay → lane
|
||||
# (operator) → caller's request-shaped default → in-code model
|
||||
# definition → None. None/unset knobs are OMITTED from the wire so
|
||||
# the inference engine's default rules (house rule: a Python-level
|
||||
# constant anywhere on this path is a hidden pin). Direct keyword
|
||||
# call (not **kwargs) so strict mypy checks the module's most
|
||||
# important invocation against the Protocol.
|
||||
# (operator) → in-code model definition → None. None/unset knobs are
|
||||
# OMITTED from the wire so the inference engine's default rules
|
||||
# (house rule: a Python-level constant anywhere on this path is a
|
||||
# hidden pin). Direct keyword call (not **kwargs) so strict mypy
|
||||
# checks the module's most important invocation against the Protocol.
|
||||
effective_effort = (
|
||||
reasoning_effort
|
||||
or lane.reasoning_effort
|
||||
or default_reasoning_effort
|
||||
or (lane.capabilities.default_reasoning_effort if lane.capabilities else None)
|
||||
or None
|
||||
)
|
||||
|
||||
@@ -539,13 +539,14 @@ class OutputGuardJudge:
|
||||
capabilities=self._capabilities,
|
||||
config_store=self._config_store,
|
||||
)
|
||||
# Temperature deliberately not pinned (house rule) — the lane
|
||||
# inherits the guard model's configured temperature. The effort
|
||||
# default is request shape, not a pin: screening runs inside a
|
||||
# 512-token cap, and an unconstrained thinking pass would consume
|
||||
# the whole budget and return empty content (the stage would
|
||||
# silently no-op to heuristic-only). Any operator or
|
||||
# model-definition effort value beats it.
|
||||
# Sampling deliberately not pinned (house rule) — the lane inherits
|
||||
# the guard model's full assignment scheme, effort included: a
|
||||
# code-chosen effort is an unvetted token on local vocabularies
|
||||
# and can flip template thinking toggles the operator never
|
||||
# engaged. On a thinking model whose effort the operator leaves
|
||||
# unbounded, a pass that consumes the whole 512-token cap parses
|
||||
# to a labelled llm_error verdict (heuristic tier stands) — the
|
||||
# remediation is an effort value on the guard's model alias.
|
||||
try:
|
||||
result = run_with_deadline(
|
||||
lambda: model_turn(
|
||||
@@ -553,7 +554,6 @@ class OutputGuardJudge:
|
||||
judge_turns,
|
||||
tools=None,
|
||||
max_tokens=512,
|
||||
default_reasoning_effort="low",
|
||||
),
|
||||
timeout=timeout,
|
||||
cancel_event=cancel_event,
|
||||
|
||||
+26
-18
@@ -828,14 +828,18 @@ _SPEC_ARGUMENTS_LITERAL_RE = re.compile(r"\$ARGUMENTS\b(?!\[)")
|
||||
# small ``max_tokens`` lets the reasoning pass swallow the entire budget so the
|
||||
# title text never lands (``finish_reason=length``, empty ``content`` → skip).
|
||||
# This hits auto-title and refresh alike (shared path) on any thinking model.
|
||||
# So give the think pass room (``_TITLE_MAX_TOKENS``), then recover the title
|
||||
# from ``content``: reuse :meth:`ChatSession._strip_reasoning` (the canonical
|
||||
# ``<think>``/``<reasoning>`` remover, for lanes that leave reasoning inline
|
||||
# rather than in ``reasoning_content``), take the first non-empty line (a model
|
||||
# that appends an explanation shouldn't fold prose into the title), then peel a
|
||||
# ``Title:`` label and wrapping markdown/quote decoration. Internal punctuation
|
||||
# is preserved so ``.NET``, ``CI/CD``, ``v1.6.0`` survive.
|
||||
_TITLE_MAX_TOKENS = 2048
|
||||
# Code deliberately does NOT bound the thinking (no effort value survives the
|
||||
# assignment scheme unless the operator set one), so the budget must fit a
|
||||
# full thinking pass at the MODEL'S OWN default — the prompt's hard word cap
|
||||
# keeps the visible answer trivially cheap, and ``_TITLE_MAX_TOKENS`` carries
|
||||
# the rest. Recover the title from ``content``: reuse
|
||||
# :meth:`ChatSession._strip_reasoning` (the canonical ``<think>``/
|
||||
# ``<reasoning>`` remover, for lanes that leave reasoning inline rather than
|
||||
# in ``reasoning_content``), take the first non-empty line (a model that
|
||||
# appends an explanation shouldn't fold prose into the title), then peel a
|
||||
# ``Title:`` label and wrapping markdown/quote decoration. Internal
|
||||
# punctuation is preserved so ``.NET``, ``CI/CD``, ``v1.6.0`` survive.
|
||||
_TITLE_MAX_TOKENS = 8192
|
||||
# Match the manual-rename (alias) cap so generated and hand-set titles share
|
||||
# one length bound.
|
||||
_TITLE_MAX_CHARS = 80
|
||||
@@ -3323,7 +3327,8 @@ class ChatSession:
|
||||
"# Instructions\n\n"
|
||||
"You are a conversation title generator. "
|
||||
"The user will show you the opening of a conversation. "
|
||||
"Respond with ONLY a short title (3-8 words). "
|
||||
"Respond with ONLY a short title of AT MOST 3 words — "
|
||||
"this is a hard rule; 2-3 words is ideal. "
|
||||
"Do NOT answer the conversation. Do NOT explain. "
|
||||
"Output ONLY the title text, nothing else."
|
||||
),
|
||||
@@ -4778,14 +4783,18 @@ class ChatSession:
|
||||
— the same operator/registry-resolved value the main turn uses — rather
|
||||
than a hard-coded constant: utility calls should not silently override an
|
||||
explicit ``[models.*]`` temperature. ``reasoning_effort`` ``None``
|
||||
inherits the lane's operator rungs, then the request-shaped
|
||||
``default_reasoning_effort="low"``: these calls run inside small
|
||||
token budgets, and an unconstrained thinking pass can consume the
|
||||
whole budget and return empty content (the #676 signature) — any
|
||||
operator or model-definition value still beats the default.
|
||||
Callers relaying the session's user-facing effort knob (web-fetch
|
||||
extraction) pass it explicitly. extra_params resolve inside the
|
||||
lane from the same single config fetch as the rest.
|
||||
inherits the lane's full assignment scheme (operator rungs → model
|
||||
definition → omit) with NO code-supplied default: a code-chosen
|
||||
effort is an unvetted token on local vocabularies and can flip
|
||||
template thinking toggles the operator never engaged. These calls
|
||||
run inside small token budgets, so on thinking models the operator
|
||||
must budget effort via the alias/model definition — an unbounded
|
||||
thinking pass consuming the whole budget surfaces as the documented
|
||||
empty-content signature (#676), and the max_tokens here are sized
|
||||
generously for exactly that reason. Callers relaying the session's
|
||||
user-facing effort knob (web-fetch extraction) pass it explicitly.
|
||||
extra_params resolve inside the lane from the same single config
|
||||
fetch as the rest.
|
||||
"""
|
||||
caps = self._get_capabilities()
|
||||
clamped = min(max_tokens, caps.max_output_tokens) if caps.max_output_tokens else max_tokens
|
||||
@@ -4804,7 +4813,6 @@ class ChatSession:
|
||||
max_tokens=clamped,
|
||||
temperature=self.temperature if temperature is None else temperature,
|
||||
reasoning_effort=reasoning_effort,
|
||||
default_reasoning_effort="low",
|
||||
)
|
||||
# Utility completions (title gen, compaction, web-fetch extraction)
|
||||
# bypass the streaming on_status path — record their usage so the
|
||||
|
||||
Reference in New Issue
Block a user