feat(daemon): add PatternDetector for recurring tool sequences and workflows (#15)#21
Merged
Merged
Conversation
…rkflows (#15) 4 detection strategies: REPEATED_TOOL_SEQUENCE (LCS-based subsequence matching with cross-session memory lookup), ERROR_CORRECTION (cross-turn aggregation), USER_PREFERENCE (Jaccard similarity grouping of corrections), WORKFLOW (recurring user prompt detection). All heuristic-based, no LLM calls.
📝 WalkthroughWalkthroughAdds a new PatternDetector class that analyzes turns and session history to detect four pattern types (REPEATED_TOOL_SEQUENCE, ERROR_CORRECTION, USER_PREFERENCE, WORKFLOW), with optional cross-session lookups via AutoMemoryStore, plus comprehensive unit tests covering detection strategies and edge cases. Changes
Sequence Diagram(s)mermaid Client->>PatternDetector: analyze(Turn, SessionHistory, ToolMetrics) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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
🧹 Nitpick comments (1)
aceclaw-daemon/src/main/java/dev/aceclaw/daemon/PatternDetector.java (1)
416-422:contentContainsSequencemay produce false positives due to substring matching.The current implementation checks if each tool name is contained as a substring in the content. This can cause false positives:
- Tool
"read"would match content containing"read_file"- Tool
"file"would match both"read_file"and"write_file"Consider using word boundary matching or checking for the exact tool name format used in stored patterns.
Possible stricter matching
private static boolean contentContainsSequence(String content, List<String> sequence) { if (content == null) return false; for (var tool : sequence) { - if (!content.contains(tool)) return false; + // Match tool name with word boundaries + if (!content.matches("(?s).*\\b" + java.util.regex.Pattern.quote(tool) + "\\b.*")) { + return false; + } } return true; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@aceclaw-daemon/src/main/java/dev/aceclaw/daemon/PatternDetector.java` around lines 416 - 422, contentContainsSequence currently uses substring matching which yields false positives (e.g., "read" matching "read_file"); change it to perform exact/token or word-boundary matching for each entry in sequence: either tokenize content (split by non-alphanumeric/underscore chars) and check for equality against each tool, or use regex with word boundaries and Pattern.quote for each tool (e.g., Pattern.compile("\\b" + Pattern.quote(tool) + "\\b")). Update the method contentContainsSequence to use one of these stricter checks and ensure it matches the exact tool name format used by stored patterns.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@aceclaw-daemon/src/main/java/dev/aceclaw/daemon/PatternDetector.java`:
- Around line 67-85: The analyze method currently declares an unused parameter
toolMetrics; either remove it or start using it: if you intend it to influence
detection, thread toolMetrics into the relevant detectors (e.g., pass it into
detectErrorCorrectionPatterns and/or detectRepeatedToolSequences and update
those method signatures and their callers to consult execution stats), otherwise
remove the toolMetrics parameter from PatternDetector.analyze and update all
callers and any tests to stop passing a ToolMetrics map; ensure compiler errors
are fixed by adjusting method signatures for
detectErrorCorrectionPatterns/detectRepeatedToolSequences when chosen.
- Around line 395-396: The use of the underscore lambda parameter in
errors.computeIfAbsent(toolName, _ -> new ArrayList<>()).add(after) is Java 22+
syntax; change the lambda to use a regular parameter name (e.g., key or k) so it
compiles on Java 21 (replace "_" with a named parameter in the computeIfAbsent
call in PatternDetector.java).
- Around line 359-381: In extractToolSequencesFromHistory, the code clears the
working list current when it finds "tool:" but never populates it, so sequences
is always empty; update the logic in extractToolSequencesFromHistory to parse
and append detected tool names into current when processing Assistant messages
(e.g., extract tokens after "tool:" or read structured tool info from
AgentSession.ConversationMessage.Assistant), only clear current when starting a
new assistant turn after adding a completed sequence to sequences, and keep the
existing check against MIN_SEQUENCE_LENGTH before adding to sequences; reference
the current variable, the sequences list, and MIN_SEQUENCE_LENGTH when making
the fix.
---
Nitpick comments:
In `@aceclaw-daemon/src/main/java/dev/aceclaw/daemon/PatternDetector.java`:
- Around line 416-422: contentContainsSequence currently uses substring matching
which yields false positives (e.g., "read" matching "read_file"); change it to
perform exact/token or word-boundary matching for each entry in sequence: either
tokenize content (split by non-alphanumeric/underscore chars) and check for
equality against each tool, or use regex with word boundaries and Pattern.quote
for each tool (e.g., Pattern.compile("\\b" + Pattern.quote(tool) + "\\b")).
Update the method contentContainsSequence to use one of these stricter checks
and ensure it matches the exact tool name format used by stored patterns.
Comment on lines
+67
to
+85
| public List<PatternInsight> analyze( | ||
| Turn turn, | ||
| List<AgentSession.ConversationMessage> sessionHistory, | ||
| Map<String, ToolMetrics> toolMetrics) { | ||
|
|
||
| var insights = new ArrayList<PatternInsight>(); | ||
|
|
||
| if (turn != null) { | ||
| insights.addAll(detectRepeatedToolSequences(turn, sessionHistory)); | ||
| insights.addAll(detectErrorCorrectionPatterns(turn, sessionHistory)); | ||
| } | ||
|
|
||
| if (sessionHistory != null && !sessionHistory.isEmpty()) { | ||
| insights.addAll(detectUserPreferences(sessionHistory)); | ||
| insights.addAll(detectWorkflows(sessionHistory)); | ||
| } | ||
|
|
||
| return List.copyOf(insights); | ||
| } |
There was a problem hiding this comment.
Unused toolMetrics parameter.
The toolMetrics parameter is declared in the method signature but never used in any of the detection strategies. This suggests either incomplete implementation or dead code.
Consider either:
- Removing the parameter if it's not needed
- Implementing its usage (e.g., incorporating execution statistics into error correction or performance pattern detection)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@aceclaw-daemon/src/main/java/dev/aceclaw/daemon/PatternDetector.java` around
lines 67 - 85, The analyze method currently declares an unused parameter
toolMetrics; either remove it or start using it: if you intend it to influence
detection, thread toolMetrics into the relevant detectors (e.g., pass it into
detectErrorCorrectionPatterns and/or detectRepeatedToolSequences and update
those method signatures and their callers to consult execution stats), otherwise
remove the toolMetrics parameter from PatternDetector.analyze and update all
callers and any tests to stop passing a ToolMetrics map; ensure compiler errors
are fixed by adjusting method signatures for
detectErrorCorrectionPatterns/detectRepeatedToolSequences when chosen.
Comment on lines
+359
to
+381
| private List<List<String>> extractToolSequencesFromHistory( | ||
| List<AgentSession.ConversationMessage> history) { | ||
| // Group by assistant turns, extract tool names from the text | ||
| var sequences = new ArrayList<List<String>>(); | ||
| var current = new ArrayList<String>(); | ||
|
|
||
| for (var msg : history) { | ||
| if (msg instanceof AgentSession.ConversationMessage.Assistant assistant) { | ||
| // Extract tool names mentioned in assistant text (heuristic) | ||
| var text = assistant.content(); | ||
| if (text != null && text.contains("tool:")) { | ||
| // Simple extraction — real implementation uses structured data | ||
| current.clear(); | ||
| } | ||
| } else if (msg instanceof AgentSession.ConversationMessage.User) { | ||
| if (current.size() >= MIN_SEQUENCE_LENGTH) { | ||
| sequences.add(List.copyOf(current)); | ||
| } | ||
| current.clear(); | ||
| } | ||
| } | ||
| return sequences; | ||
| } |
There was a problem hiding this comment.
extractToolSequencesFromHistory always returns empty sequences.
The method clears current at line 371 but never adds anything to it. The only path that could add to sequences (line 375) requires current.size() >= MIN_SEQUENCE_LENGTH, which will never be true since current is always empty.
This makes cross-session detection from session history ineffective. Either this is incomplete implementation, or the intent was different.
Possible fix if structured data is available
private List<List<String>> extractToolSequencesFromHistory(
List<AgentSession.ConversationMessage> history) {
- // Group by assistant turns, extract tool names from the text
var sequences = new ArrayList<List<String>>();
var current = new ArrayList<String>();
for (var msg : history) {
if (msg instanceof AgentSession.ConversationMessage.Assistant assistant) {
- // Extract tool names mentioned in assistant text (heuristic)
var text = assistant.content();
- if (text != null && text.contains("tool:")) {
- // Simple extraction — real implementation uses structured data
- current.clear();
+ if (text != null) {
+ // Heuristic: extract tool names from patterns like "tool: <name>" or "[<name>]"
+ var matcher = java.util.regex.Pattern.compile("tool:\\s*(\\w+)|\\[(\\w+)\\]")
+ .matcher(text);
+ while (matcher.find()) {
+ String toolName = matcher.group(1) != null ? matcher.group(1) : matcher.group(2);
+ if (toolName != null) current.add(toolName);
+ }
}
} else if (msg instanceof AgentSession.ConversationMessage.User) {
if (current.size() >= MIN_SEQUENCE_LENGTH) {
sequences.add(List.copyOf(current));
}
current.clear();
}
}
+ // Don't forget trailing sequence
+ if (current.size() >= MIN_SEQUENCE_LENGTH) {
+ sequences.add(List.copyOf(current));
+ }
return sequences;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@aceclaw-daemon/src/main/java/dev/aceclaw/daemon/PatternDetector.java` around
lines 359 - 381, In extractToolSequencesFromHistory, the code clears the working
list current when it finds "tool:" but never populates it, so sequences is
always empty; update the logic in extractToolSequencesFromHistory to parse and
append detected tool names into current when processing Assistant messages
(e.g., extract tokens after "tool:" or read structured tool info from
AgentSession.ConversationMessage.Assistant), only clear current when starting a
new assistant turn after adding a completed sequence to sequences, and keep the
existing check against MIN_SEQUENCE_LENGTH before adding to sequences; reference
the current variable, the sequences list, and MIN_SEQUENCE_LENGTH when making
the fix.
Comment on lines
+395
to
+396
| errors.computeIfAbsent(toolName, _ -> new ArrayList<>()).add(after); | ||
| } |
There was a problem hiding this comment.
Same Java version concern: unnamed variable _.
This is another instance of the unnamed variable _ syntax (Java 22+). Apply the same fix if Java 21 compatibility is required.
- errors.computeIfAbsent(toolName, _ -> new ArrayList<>()).add(after);
+ errors.computeIfAbsent(toolName, k -> new ArrayList<>()).add(after);📝 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.
Suggested change
| errors.computeIfAbsent(toolName, _ -> new ArrayList<>()).add(after); | |
| } | |
| errors.computeIfAbsent(toolName, k -> new ArrayList<>()).add(after); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@aceclaw-daemon/src/main/java/dev/aceclaw/daemon/PatternDetector.java` around
lines 395 - 396, The use of the underscore lambda parameter in
errors.computeIfAbsent(toolName, _ -> new ArrayList<>()).add(after) is Java 22+
syntax; change the lambda to use a regular parameter name (e.g., key or k) so it
compiles on Java 21 (replace "_" with a named parameter in the computeIfAbsent
call in PatternDetector.java).
ConversationMessage is text-only with no structured tool data. Cross-session sequence detection uses AutoMemoryStore instead.
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@aceclaw-daemon/src/main/java/dev/aceclaw/daemon/PatternDetector.java`:
- Around line 105-112: The loop over priorPatterns uses contentContainsSequence
which only tests for presence of tool names and can yield false positives;
change the matching to validate order by using the existing isSubsequenceMatch
helper (or by parsing stored MemoryEntry content into a sequence and performing
an ordered subsequence check) when comparing each entry to currentSeq; update
the block that increments frequency and adds evidence (the code touching
MemoryEntry, currentSeq, frequency, evidence, truncate, and memoryStore.query)
to only count and record matches when isSubsequenceMatch confirms the stored
sequence is an ordered subsequence of currentSeq.
- Around line 143-146: The use of the unnamed lambda parameter `_` in the
computeIfAbsent call inside PatternDetector is not Java 21 compatible; update
the computeIfAbsent call on errorTools to use a named parameter (e.g., key or k)
instead of `_` so the lambda is valid in Java 21—locate the block where
ContentBlock.ToolResult tr is handled (the computeIfAbsent call referencing
findToolName(turn.newMessages(), tr.toolUseId())) and replace the underscore
lambda parameter with a simple named parameter like k -> new ArrayList<>().
---
Duplicate comments:
In `@aceclaw-daemon/src/main/java/dev/aceclaw/daemon/PatternDetector.java`:
- Around line 358-361: The lambda in PatternDetector using
errors.computeIfAbsent(toolName, _ -> new ArrayList<>()) uses the unnamed
parameter "_" which is not Java 21 compatible; change the lambda to use a real
identifier (e.g., key -> new ArrayList<>()) so replace "_" with a named
parameter in the computeIfAbsent call where toolName is the key and errors is
populated (same pattern as the earlier fix at line 145).
- Around line 67-70: The analyze method in PatternDetector declares a
toolMetrics parameter that is never used; either remove it from
PatternDetector.analyze (and all callers such as where
AgentSession.ConversationMessage lists are passed) or thread it into your
detection flow by updating each strategy/collector so they accept Map<String,
ToolMetrics> (or expose a helper that consumes it) and use metrics to weight or
filter PatternInsight results (e.g., scale confidence by execution time/failure
rates from ToolMetrics). Update the PatternDetector.analyze signature, all
callers, and any Strategy/Detector interfaces (and tests) consistently, and
ensure PatternInsight creation includes the metric-influenced score when you
choose to incorporate it.
Comment on lines
+105
to
+112
| var priorPatterns = memoryStore.query( | ||
| MemoryEntry.Category.PATTERN, List.of("tool-sequence"), 0); | ||
| for (var entry : priorPatterns) { | ||
| if (contentContainsSequence(entry.content(), currentSeq)) { | ||
| frequency++; | ||
| evidence.add("memory: " + truncate(entry.content(), 100)); | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential false positives in cross-session sequence matching.
contentContainsSequence only checks that each tool name appears somewhere in the content string, ignoring order. A stored entry with "C -> B -> A" would incorrectly match a current sequence of [A, B, C].
Consider using the existing isSubsequenceMatch helper or storing sequences in a parseable format to validate order.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@aceclaw-daemon/src/main/java/dev/aceclaw/daemon/PatternDetector.java` around
lines 105 - 112, The loop over priorPatterns uses contentContainsSequence which
only tests for presence of tool names and can yield false positives; change the
matching to validate order by using the existing isSubsequenceMatch helper (or
by parsing stored MemoryEntry content into a sequence and performing an ordered
subsequence check) when comparing each entry to currentSeq; update the block
that increments frequency and adds evidence (the code touching MemoryEntry,
currentSeq, frequency, evidence, truncate, and memoryStore.query) to only count
and record matches when isSubsequenceMatch confirms the stored sequence is an
ordered subsequence of currentSeq.
Comment on lines
+143
to
+146
| if (block instanceof ContentBlock.ToolResult tr && tr.isError()) { | ||
| errorTools.computeIfAbsent(findToolName(turn.newMessages(), tr.toolUseId()), | ||
| _ -> new ArrayList<>()).add(tr.content()); | ||
| } |
There was a problem hiding this comment.
Java 21 compatibility: unnamed variable _ is Java 22+ syntax.
The underscore _ as an unnamed lambda parameter was finalized in Java 22 (JEP 456). Per coding guidelines requiring Java 21 API compatibility, use a named parameter.
🔧 Proposed fix
if (block instanceof ContentBlock.ToolResult tr && tr.isError()) {
errorTools.computeIfAbsent(findToolName(turn.newMessages(), tr.toolUseId()),
- _ -> new ArrayList<>()).add(tr.content());
+ k -> new ArrayList<>()).add(tr.content());
}📝 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.
Suggested change
| if (block instanceof ContentBlock.ToolResult tr && tr.isError()) { | |
| errorTools.computeIfAbsent(findToolName(turn.newMessages(), tr.toolUseId()), | |
| _ -> new ArrayList<>()).add(tr.content()); | |
| } | |
| if (block instanceof ContentBlock.ToolResult tr && tr.isError()) { | |
| errorTools.computeIfAbsent(findToolName(turn.newMessages(), tr.toolUseId()), | |
| k -> new ArrayList<>()).add(tr.content()); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@aceclaw-daemon/src/main/java/dev/aceclaw/daemon/PatternDetector.java` around
lines 143 - 146, The use of the unnamed lambda parameter `_` in the
computeIfAbsent call inside PatternDetector is not Java 21 compatible; update
the computeIfAbsent call on errorTools to use a named parameter (e.g., key or k)
instead of `_` so the lambda is valid in Java 21—locate the block where
ContentBlock.ToolResult tr is handled (the computeIfAbsent call referencing
findToolName(turn.newMessages(), tr.toolUseId())) and replace the underscore
lambda parameter with a simple named parameter like k -> new ArrayList<>().
This was referenced Feb 19, 2026
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PatternDetectorwith 4 heuristic detection strategies (no LLM calls):REPEATED_TOOL_SEQUENCE— LCS-based subsequence matching, cross-session via AutoMemoryStoreERROR_CORRECTION— cross-turn error aggregation per tool nameUSER_PREFERENCE— Jaccard similarity grouping of user corrections (threshold 0.6)WORKFLOW— recurring user prompt detection (threshold 0.5)extractToolSequence,isSubsequenceMatch,jaccardSimilarityTest plan
PatternDetectorTest./gradlew clean build— 64 tasks, BUILD SUCCESSFUL)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests