feat(toolset): trigger cross-step dedup reminder only after 7 consecutive repeats#2280
feat(toolset): trigger cross-step dedup reminder only after 7 consecutive repeats#2280jackfish212 wants to merge 2 commits into
Conversation
…tive repeats - add consecutive call counter to track repeat occurrences across steps\n- trigger cross-step dedup reminder only on 7th consecutive identical call\n- reset counter when a different tool or arguments intervene\n- update tests for new threshold and reset behavior
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e5677b9c0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if pending_part is not None: | ||
| _message_append(message, pending_part) | ||
| if isinstance(pending_part, ToolCall) and on_tool_call: | ||
| await callback(on_tool_call, pending_part) |
There was a problem hiding this comment.
Avoid duplicating tool calls when cancellation hits callback
If the task is cancelled while awaiting on_tool_call after _message_append has already appended pending_part, the except asyncio.CancelledError block re-appends the same pending_part because it is not cleared until after the callback returns. In that cancellation window (including async/custom on_tool_call callbacks), the StepCancelled partial contains duplicate assistant tool calls with the same id, and KimiSoul will persist/send duplicate paired results, leaving malformed history for the next model call.
Useful? React with 👍 / 👎.
- record interrupted tool results in context when step is cancelled\n- send ToolError with "Interrupted By User" for pending tool calls\n- add unit and e2e tests for tool interrupt scenarios\n- suppress auto-completion when browsing recalled history entries in shell UI
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb4dece9bd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| async for part in stream: | ||
| logger.trace("Received part: {part}", part=part) | ||
| if on_message_part: | ||
| await callback(on_message_part, part.model_copy(deep=True)) |
There was a problem hiding this comment.
Record streamed parts before awaiting callbacks
When on_message_part is an async callback, a user cancellation delivered while this await is in progress raises GenerateCancelled before the newly streamed part has been merged into pending_part or message. In that scenario the callback may already have rendered a text/tool-call chunk to the UI, but the partial result used to repair history omits it, so a visible tool call can still be persisted without the required synthetic tool result (or lost entirely).
Useful? React with 👍 / 👎.
Summary
This PR raises the threshold for triggering cross-step deduplication reminders from "any repeat" to "7 consecutive identical calls". This reduces noise from legitimate repeated tool usage while still surfacing stuck-loop behavior to the model.
1. Cross-step dedup reminder threshold
Problem: Previously, any tool call identical to one in the previous step would trigger a deduplication reminder. This was overly aggressive for workflows that legitimately need to call the same tool with the same arguments multiple times across steps (e.g., polling, incremental reads).
What was done:
_CROSS_STEP_DEDUP_TRIGGER_COUNT = 7constant._consecutive_call_keyand_consecutive_call_counttoKimiToolsetto track how many times the exact same(tool_name, arguments)pair has been called consecutively across steps._sync_consecutive_state_from_previous_calls()to initialize the counter from the previous step'''s call history._projected_consecutive_state()to evaluate whether the current call would hit the threshold after accounting for calls already made in the current step.2. Counter reset behavior
Problem: The consecutive counter needs to reset when a different tool or different arguments intervene, otherwise unrelated calls could inadvertently contribute to the threshold.
What was done:
(tool_name, arguments)key appears.begin_step/end_stepstate tracking) and within the same step (via_projected_consecutive_state).3. Test coverage
What was done:
test_cross_step_duplicate_appends_reminderto verify the reminder appears on the 7th occurrence.test_cross_step_repeat_counter_resets_after_other_tool_callto verify an intervening different tool call resets the counter.test_cross_step_repeat_counter_resets_with_intervening_call_in_same_stepto verify a different call within the same step also resets the counter.Checklist
make gen-changelogto update the changelog.make gen-docsto update the user documentation.