test + docs: lock should_execute_tools guard semantics (#3220)
Two small follow-ups to the guard:
1. Fix the should_execute_tools docstring so it matches the actual code.
The previous version said "Only execute when finish_reason explicitly
signals tool intent" but the code also accepts finish_reason == "stop".
Explain why (some compliant providers emit "stop" with legitimate tool
calls — openai_compat_provider.py already mirrors this at lines ~633 /
~678 where ("tool_calls", "stop") are both treated as the terminal
tool-call state). Without this, a strict "tool_calls"-only guard would
regress 15 existing runner tests that construct LLMResponse with
tool_calls but no explicit finish_reason (default = "stop").
2. Add tests/providers/test_llm_response.py. This locks the three cases:
- no tool calls -> never executes
- tool calls + "tool_calls"/stop -> executes
- tool calls + refusal / content_filter / error / length / ... -> blocked
These are exactly the boundary cases the #3220 fix is about; without a
test here a future refactor could silently revert the guard.
Body + tests only, no behavior change beyond the existing PR's intent.
Made-with: Cursor
This commit is contained in:
@@ -75,17 +75,18 @@ class LLMResponse:
|
||||
def should_execute_tools(self) -> bool:
|
||||
"""Check if tool calls should be executed (guards against gateway injection).
|
||||
|
||||
Only execute when finish_reason explicitly signals tool intent.
|
||||
Tool calls under any other finish_reason (refusal, content_filter, error, etc.)
|
||||
are treated as anomalous and should not be executed.
|
||||
Executes only when ``has_tool_calls`` is true and ``finish_reason`` is one of
|
||||
the known-good signals: ``"tool_calls"`` (explicit intent) or ``"stop"`` (some
|
||||
compliant providers emit ``stop`` for legitimate tool calls; existing paths in
|
||||
``openai_compat_provider`` already treat both as the tool-call terminal state).
|
||||
|
||||
Tool calls under any other ``finish_reason`` (e.g. ``refusal``, ``content_filter``,
|
||||
``error``) are treated as anomalous — typically injected by non-compliant API
|
||||
gateways — and are skipped.
|
||||
"""
|
||||
if not self.has_tool_calls:
|
||||
return False
|
||||
if self.finish_reason == "tool_calls":
|
||||
return True
|
||||
if self.finish_reason == "stop":
|
||||
return True
|
||||
return False
|
||||
return self.finish_reason in ("tool_calls", "stop")
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
|
||||
Reference in New Issue
Block a user