Skip to content

Commit 0f2dd93

Browse files
committed
fix: address CodeRabbit review on dedup reminder and test
Bound the canonical arguments echoed in the strong dedup reminder to a 256-char preview so large-payload tools (WriteFile, MultiEdit) don't re-inject their whole body into context on every repeat; exact identity is still carried by the args_hash dedup telemetry. Rewrite test_begin_end_step to assert observable behaviour (handle()/end_step()/dedup_triggered) instead of poking private _current_step_* internals.
1 parent f951b26 commit 0f2dd93

2 files changed

Lines changed: 16 additions & 11 deletions

File tree

‎src/pythinker_code/soul/toolset.py‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,13 +228,23 @@ def type_check(pythinker_toolset: PythinkerToolset):
228228

229229

230230
def _make_reminder_text_2(tool_name: str, repeat_count: int, canonical_args: str) -> str:
231+
# Echo only a bounded preview of the arguments: large-payload tools
232+
# (WriteFile, MultiEdit) would otherwise re-inject the whole body into
233+
# context on every repeat — defeating the reminder by inflating tokens.
234+
# Exact identity is preserved by the args_hash in the dedup telemetry.
235+
args_limit = 256
236+
if len(canonical_args) > args_limit:
237+
dropped = len(canonical_args) - args_limit
238+
args_preview = f"{canonical_args[:args_limit]}... [truncated {dropped} chars]"
239+
else:
240+
args_preview = canonical_args
231241
return (
232242
"\n\n<system-reminder>\n"
233243
"You have repeatedly called the same tool with identical parameters many times.\n"
234244
"Repeated tool call detected:\n"
235245
f"- tool: {tool_name}\n"
236246
f"- repeated_times: {repeat_count}\n"
237-
f"- arguments: {canonical_args}\n"
247+
f"- arguments: {args_preview}\n"
238248
"The previous repeated calls did not make progress. Do not call this exact same tool "
239249
"with the exact same arguments again.\n"
240250
"Carefully inspect the latest tool result and choose a different next action, "

‎tests/core/test_toolset.py‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -449,22 +449,17 @@ async def test_non_duplicate_allowed():
449449
assert ts.end_step() == [("ToolA", '{"value":"y"}')]
450450

451451

452-
def test_begin_end_step():
453-
"""begin_step and end_step should correctly manage deduplication state."""
452+
async def test_begin_end_step():
453+
"""begin_step seeds the prior step's calls; end_step captures this step's."""
454454
ts = _make_toolset()
455455

456456
ts.begin_step([("ToolA", "{}")])
457-
assert ts._previous_step_calls == [("ToolA", "{}")]
458-
assert ts._current_step_calls == []
459-
assert ts._current_step_tasks == {}
460457
assert ts.dedup_triggered is False
461458

462-
ts._current_step_calls.append(("ToolB", "{}"))
459+
# A fresh (non-duplicate) call this step is captured by end_step() and does
460+
# not trip cross-step dedup, since only ToolA was seen previously.
461+
await ts.handle(ToolCall(id="b1", function=ToolCall.FunctionBody(name="ToolB", arguments="{}")))
463462
assert ts.end_step() == [("ToolB", "{}")]
464-
465-
# After end_step, internal lists are not cleared by end_step itself;
466-
# the caller (PythinkerSoul) is expected to call begin_step again for the next step.
467-
# But dedup_triggered should still reflect the last step's state.
468463
assert ts.dedup_triggered is False
469464

470465

0 commit comments

Comments
 (0)