Implement RecursionDepthDetector for cognitive loop detection#669
Conversation
|
@Akshaya-125 is attempting to deploy a commit to the sreerevanth's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds ChangesRecursion detection
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant AgentEvent
participant RecursionDepthDetector
participant RecursionDepthReport
AgentEvent->>RecursionDepthDetector: observe planner-output preview
RecursionDepthDetector->>RecursionDepthDetector: buffer and scan configured patterns
RecursionDepthDetector-->>RecursionDepthReport: return detection result
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agentwatch/core/recursion_depth_detector.py`:
- Around line 36-47: Validate buffer_size and threshold at the start of
RecursionDepthDetector.__init__, rejecting values below 1 and 0 respectively
before compiling patterns or constructing the deque. Preserve valid
configurations and raise an appropriate value-related exception for invalid
inputs.
- Around line 62-66: Update the matching loop in the recursion-depth detector to
count every occurrence of each pattern within each buffered text preview, rather
than only the first match returned by pattern.search(text). Preserve recording
the matched pattern for each occurrence and ensure the accumulated count can
exceed the configured threshold so detected is set correctly.
- Around line 43-45: Update the pattern selection in the recursion depth
detector initializer to use DEFAULT_PATTERNS only when patterns is None,
preserving an explicitly provided empty list as empty. Keep the existing regex
compilation behavior unchanged for supplied patterns.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 772d9736-5628-40c5-aa8d-0585955618f5
📒 Files selected for processing (2)
agentwatch/core/recursion_depth_detector.pytests/test_safety.py
| def __init__( | ||
| self, | ||
| buffer_size: int = DEFAULT_BUFFER_SIZE, | ||
| threshold: int = DEFAULT_THRESHOLD, | ||
| patterns: list[str] | None = None, | ||
| ): | ||
| self.threshold = threshold | ||
| self.patterns = [ | ||
| re.compile(p, re.IGNORECASE) | ||
| for p in (patterns or DEFAULT_PATTERNS) | ||
| ] | ||
| self.buffer = deque(maxlen=buffer_size) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject invalid detector configuration.
buffer_size=0 creates a deque that silently discards every observation, while threshold < 0 makes any planner event with zero matches detected. Validate buffer_size >= 1 and threshold >= 0 before constructing the detector.
Proposed fix
):
+ if buffer_size < 1:
+ raise ValueError("buffer_size must be at least 1")
+ if threshold < 0:
+ raise ValueError("threshold must be non-negative")
self.threshold = threshold📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def __init__( | |
| self, | |
| buffer_size: int = DEFAULT_BUFFER_SIZE, | |
| threshold: int = DEFAULT_THRESHOLD, | |
| patterns: list[str] | None = None, | |
| ): | |
| self.threshold = threshold | |
| self.patterns = [ | |
| re.compile(p, re.IGNORECASE) | |
| for p in (patterns or DEFAULT_PATTERNS) | |
| ] | |
| self.buffer = deque(maxlen=buffer_size) | |
| def __init__( | |
| self, | |
| buffer_size: int = DEFAULT_BUFFER_SIZE, | |
| threshold: int = DEFAULT_THRESHOLD, | |
| patterns: list[str] | None = None, | |
| ): | |
| if buffer_size < 1: | |
| raise ValueError("buffer_size must be at least 1") | |
| if threshold < 0: | |
| raise ValueError("threshold must be non-negative") | |
| self.threshold = threshold | |
| self.patterns = [ | |
| re.compile(p, re.IGNORECASE) | |
| for p in (patterns or DEFAULT_PATTERNS) | |
| ] | |
| self.buffer = deque(maxlen=buffer_size) |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 43-43: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.compile(p, re.IGNORECASE)
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-python)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agentwatch/core/recursion_depth_detector.py` around lines 36 - 47, Validate
buffer_size and threshold at the start of RecursionDepthDetector.__init__,
rejecting values below 1 and 0 respectively before compiling patterns or
constructing the deque. Preserve valid configurations and raise an appropriate
value-related exception for invalid inputs.
| self.patterns = [ | ||
| re.compile(p, re.IGNORECASE) | ||
| for p in (patterns or DEFAULT_PATTERNS) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Honor an explicitly empty pattern list.
patterns or DEFAULT_PATTERNS replaces [] with the defaults, so callers cannot intentionally configure a detector with no patterns. Use patterns is None to distinguish omission from an empty configuration.
Proposed fix
- for p in (patterns or DEFAULT_PATTERNS)
+ for p in (DEFAULT_PATTERNS if patterns is None else patterns)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| self.patterns = [ | |
| re.compile(p, re.IGNORECASE) | |
| for p in (patterns or DEFAULT_PATTERNS) | |
| self.patterns = [ | |
| re.compile(p, re.IGNORECASE) | |
| for p in (DEFAULT_PATTERNS if patterns is None else patterns) |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 43-43: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.compile(p, re.IGNORECASE)
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-python)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agentwatch/core/recursion_depth_detector.py` around lines 43 - 45, Update the
pattern selection in the recursion depth detector initializer to use
DEFAULT_PATTERNS only when patterns is None, preserving an explicitly provided
empty list as empty. Keep the existing regex compilation behavior unchanged for
supplied patterns.
| for text in self.buffer: | ||
| for pattern in self.patterns: | ||
| if pattern.search(text): | ||
| count += 1 | ||
| matched.append(pattern.pattern) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Count every pattern occurrence in each preview.
pattern.search(text) contributes at most one match per pattern per buffered output. A preview containing repeated self-reflective phrases is therefore undercounted, allowing detected to remain false despite exceeding the configured match count.
Proposed fix
- if pattern.search(text):
+ for _ in pattern.finditer(text):
count += 1
matched.append(pattern.pattern)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for text in self.buffer: | |
| for pattern in self.patterns: | |
| if pattern.search(text): | |
| count += 1 | |
| matched.append(pattern.pattern) | |
| for text in self.buffer: | |
| for pattern in self.patterns: | |
| for _ in pattern.finditer(text): | |
| count += 1 | |
| matched.append(pattern.pattern) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agentwatch/core/recursion_depth_detector.py` around lines 62 - 66, Update the
matching loop in the recursion-depth detector to count every occurrence of each
pattern within each buffered text preview, rather than only the first match
returned by pattern.search(text). Preserve recording the matched pattern for
each occurrence and ensure the accumulated count can exceed the configured
threshold so detected is set correctly.
SHAURYASANYAL3
left a comment
SHAURYASANYAL3
left a comment
There was a problem hiding this comment.
Looks good, very clean and adheres to V2 constraints. Approving!
2 participants
Summary
This PR implements the
RecursionDepthDetectorto identify excessive self-referential reasoning that may indicate an agent is stuck in a cognitive recursion loop.Changes
RecursionDepthDetector.Testing
tests/test_safety.py.Related to #663
Summary by CodeRabbit
New Features
Tests