-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
Studio: simplify tool-call dedup and replace html2text with builtin converter #4722
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
d9380bb
cc8fbbd
dff2fd3
40cd783
b304264
70341be
2046c5c
c99f828
2c27165
f779091
ad18902
b07afb5
d398489
24b857f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,6 @@ | |
|
|
||
| import atexit | ||
| import contextlib | ||
| import hashlib | ||
| import json | ||
| import re | ||
| import struct | ||
|
|
@@ -2181,22 +2180,6 @@ def _strip_tool_markup(text: str, *, final: bool = False) -> str: | |
| # identical call succeeded). | ||
| _tool_call_history: list[tuple[str, bool]] = [] # (key, failed) | ||
|
|
||
| def _tool_call_key(name: str, args: dict) -> str: | ||
| raw = json.dumps({"t": name, "a": args}, sort_keys = True) | ||
| return hashlib.md5(raw.encode()).hexdigest() | ||
|
|
||
| def _is_duplicate_call(name: str, args: dict) -> bool: | ||
| """Block if the immediately previous call was identical and succeeded.""" | ||
| if not _tool_call_history: | ||
| return False | ||
| key = _tool_call_key(name, args) | ||
| last_key, last_failed = _tool_call_history[-1] | ||
| return last_key == key and not last_failed | ||
|
|
||
| def _record_tool_call(name: str, args: dict, failed: bool) -> None: | ||
| key = _tool_call_key(name, args) | ||
| _tool_call_history.append((key, failed)) | ||
|
|
||
| for iteration in range(max_tool_iterations): | ||
| if cancel_event is not None and cancel_event.is_set(): | ||
| return | ||
|
|
@@ -2692,7 +2675,9 @@ def _record_tool_call(name: str, args: dict, failed: bool) -> None: | |
| } | ||
|
|
||
| # ── Duplicate call detection ────────────── | ||
| if _is_duplicate_call(tool_name, arguments): | ||
| _tc_key = tool_name + str(arguments) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The change from json.dumps(..., sort_keys=True) to str(arguments) introduces a regression in robustness. While Python 3.7+ dictionaries preserve insertion order, str(dict) is sensitive to that order. If the model generates the same tool arguments but with keys in a different order, they will be semantically identical but produce different strings, causing the duplicate detection to fail. I recommend using json.dumps with sort_keys=True for the key generation. _tc_key = tool_name + json.dumps(arguments, sort_keys=True)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch -- switched to
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reverted back to |
||
| _prev = _tool_call_history[-1] if _tool_call_history else None | ||
| if _prev and _prev[0] == _tc_key and not _prev[1]: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Building the dedup key with Useful? React with 👍 / 👎. |
||
| result = ( | ||
| "You already made this exact call. " | ||
| "Do not repeat the same tool call. " | ||
|
|
@@ -2734,7 +2719,7 @@ def _record_tool_call(name: str, args: dict, failed: bool) -> None: | |
| _is_error = isinstance(result, str) and result.lstrip().startswith( | ||
| _error_prefixes | ||
| ) | ||
| _record_tool_call(tool_name, arguments, failed = _is_error) | ||
| _tool_call_history.append((_tc_key, _is_error)) | ||
| _result_content = result | ||
| if _is_error: | ||
| _result_content = ( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Building
_tc_keywithtool_name + str(arguments)makes duplicate detection depend on JSON key insertion order, so the same semantic tool call can evade dedup when the model emits arguments in a different key order on the next turn (e.g.,{\"q\":...,\"limit\":...}vs{\"limit\":...,\"q\":...}). In that case the guard no longer blocks repeated successful calls, which can re-trigger expensive or side-effectful tools and reintroduce loop behavior this block is meant to prevent.Useful? React with 👍 / 👎.