feat: extend the tool to call the duplicate detection function to support pattern duplicate detection

[aheizi]

Related GitHub Issue

Closes #5496

Description

The Tool Repetition Detector class has been enhanced and the pattern repetition detection function has been added, which can detect repetitive patterns like "ABCABC"

Test Procedure

unit test

• Test the simple alternating mode (ABABAB)
• Test the complex three-element pattern (ABCABCABC)
• Test the pattern detection of different parameter values
• Test the repetition limit of the custom mode

Pre-Submission Checklist

• Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
• Scope: My changes are focused on the linked issue (one major feature/fix per PR).
• Self-Review: I have performed a thorough self-review of my code.
• Testing: New and/or updated tests have been added to cover my changes (if applicable).
• Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
• Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

Documentation Updates

Additional Notes

Get in Touch

aheizi

---

Important

Enhance ToolRepetitionDetector to detect pattern repetitions and add corresponding tests.

• Behavior:
  • Extend ToolRepetitionDetector to detect pattern repetitions like "ABCABC" in ToolRepetitionDetector.ts.
  • Introduce historyMaxLength and patternRepetitionLimit parameters in constructor.
  • Add updateHistory(), detectPatternRepetition(), and hasRepeatingPattern() methods for pattern detection.
  • Reset state after detection using resetState().
• Tests:
  • Add tests in ToolRepetitionDetector.spec.ts for pattern detection (e.g., "ABABAB", "ABCABCABC").
  • Test different parameter values and custom repetition limits.
  • Ensure state reset after pattern detection.

^{This description was created by for bd331ed788f133f80b488cce4f914a2f47b8c99a. You can customize this summary. It will automatically update as commits are pushed.}

[daniel-lxs]

Hey @aheizi Thank you for your contribution, I think this is an interesting approach to handling more complex forms of looping.

I left some suggestions that are worth taking a look at.

Let me know if you have any questions!

[aheizi]

Hey @aheizi Thank you for your contribution, I think this is an interesting approach to handling more complex forms of looping.

I left some suggestions that are worth taking a look at.

Let me know if you have any questions!

@daniel-lxs Thank you for the review. I have revised and submitted it.

[daniel-lxs]

Thank you @aheizi, I left a couple of suggestions to help improve your implementation.

Let me know what you think!

[daniel-lxs]

I noticed that after pattern detection triggers and resets, the next tool call still fails (line 468-469). Is this intentional?

If the goal of resetState() is to allow recovery after hitting a limit, shouldn't the next call be allowed? The current behavior could trap users in a situation where they can't recover from pattern detection.

Could you clarify the intended behavior here? If this is intentional, it might be worth adding a comment explaining why.

[aheizi]

Added a comment explaining why.

[daniel-lxs]

Consider adding parameter validation in the constructor. This would prevent issue if someone passes negative values or extremely large values for historyLength or patternLimit.

For example:

constructor(consecutiveLimit: number = 3, historyLength: number = 20, patternLimit: number = 2) {
    this.consecutiveIdenticalToolCallLimit = Math.max(0, consecutiveLimit)
    this.historyMaxLength = Math.max(1, Math.min(historyLength, 1000)) // reasonable upper bound
    this.patternRepetitionLimit = Math.max(0, patternLimit)
}

This would prevent potential issues with invalid parameters.

[aheizi]

ok

[daniel-lxs]

When consecutiveIdenticalToolCallLimit is 0 (unlimited), the code skips all repetition checks including pattern detection. Is this intentional?

It seems like pattern repetition (ABABAB) could still be relevant even when consecutive repetitions (AAA) are unlimited. Should these two types of detection be independent?

If the current behavior is intentional, could you add a comment clarifying that setting consecutive limit to 0 disables ALL repetition detection, not just consecutive?

[aheizi]

Added a comment clarifying that setting consecutive limit to 0 disables ALL repetition detection.

[daniel-lxs]

The magic number 5 for history retention should be extracted to a constant or made configurable for better maintainability.

Consider:

private static readonly HISTORY_RETENTION_ON_RESET = 5;

private resetState(): void {
    this.consecutiveIdenticalToolCallCount = 0
    this.previousToolCallJson = null
    // Keep last N entries for context
    this.toolCallHistory = this.toolCallHistory.slice(-ToolRepetitionDetector.HISTORY_RETENTION_ON_RESET)
}

This makes it easier to adjust the value if needed and clarifies the intent.

[aheizi]

ok

[aheizi]

@daniel-lxs Thank you for the review. I corrected some issues and adjusted some of the annotations to make them more accurate.

[aheizi]

@daniel-lxs There’s another issue: currently, consecutiveLimit can be set to 1, which means the tool gets blocked on its first invocation. However, this doesn’t align well with the message shown to users:

Number of consecutive errors or repeated actions allowed before the “WeCoder encountered a problem” dialog appears

Since there’s no repetition at that point, the behavior feels inconsistent.

There are three possible solutions:

1. Clearly define that consecutiveLimit = 1 means “allow 1 repeated call, block on the 2nd identical call”; consecutiveLimit = 2 means “allow 2 repeated calls, block on the 3rd”, and so on.
2. When users set the value to 1, clearly inform them: “First-time invocation restriction is enabled. Every call will be restricted.”
3. Disallow setting consecutiveLimit to 1.

I’m leaning towards option 2. If you’re also okay with it, I’ll add a user-facing message accordingly. Looking forward to your feedback.

[daniel-lxs]

Hey @aheizi Good catch. I agree a message is the right approach here. That said, I think it should be clearer. “First-time invocation restriction is enabled” doesn’t make it obvious that setting consecutiveLimit to 1 means the tool will stop immediately after the first error, even if it’s not repeated. We should make that behavior explicit so users know it’s not about repetition at that point, but a strict 1-error threshold.

[aheizi]

Hey @aheizi Good catch. I agree a message is the right approach here. That said, I think it should be clearer. “First-time invocation restriction is enabled” doesn’t make it obvious that setting consecutiveLimit to 1 means the tool will stop immediately after the first error, even if it’s not repeated. We should make that behavior explicit so users know it’s not about repetition at that point, but a strict 1-error threshold.

Hey @daniel-lxs
The current implementation, setting consecutiveLimit to 1, means that each tool call will be judged as a duplicate and fail.

This is the performance of the todo list for the first time after it is set to 1：

[aheizi]

Or like this:
⚠️ Any tool call will be immediately interrupted. It is recommended to use it only during debugging.

[daniel-lxs]

Hey @aheizi, thank you for your work on this. After careful consideration, I believe this change could affect valid repetitive workflows that shouldn't be flagged as looping, such as editing files and running tests.

This doesn't mean your implementation isn't solid or that we don't appreciate it. I'm going to close the PR for now, but I'm open to continuing the discussion.

[aheizi]

@daniel-lxs The problem of repeatedly calling tools that I am currently encountering is quite frequent. Although I am also using the cloude4 model, it is still very difficult to avoid the situation of pattern repetition. The situation you mentioned where there might be misjudgments when editing files and running tests, I think, also exists in the previous consecutive repetitive scenarios. Anyway, I still very much hope that this problem can be solved from an engineering perspective.