-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: Implement intelligent semantic rule injection for context-aware rule selection #6713
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… rule selection - Add SmartRule interface and types for rule metadata - Implement smart rule loading from .roo/smart-rules directories - Create semantic matching using Jaccard similarity algorithm - Integrate smart rules into prompt generation system - Add VSCode configuration settings for smart rules - Support mode-specific smart rules directories - Add comprehensive unit tests for all functionality - Update documentation with smart rules usage Fixes #6707
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed my own code and found several areas that need attention. The implementation is solid but requires some refinements.
| .toLowerCase() | ||
| .replace(/[^\w\s]/g, " ") // Replace punctuation with spaces | ||
| .split(/\s+/) | ||
| .filter((token) => token.length > 2) // Filter out very short tokens |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intentional? The filter token.length > 2 excludes important technical terms like "AI", "ML", "DB", "UI", etc. Consider adjusting to token.length > 1 or removing the filter entirely for technical contexts:
| .filter((token) => token.length > 2) // Filter out very short tokens | |
| .filter((token) => token.length > 1) // Filter out very short tokens |
| .reverse() | ||
| .find((msg) => msg.role === "user") | ||
|
|
||
| if (lastUserMessage && Array.isArray(lastUserMessage.content)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing error handling here. The content blocks could be undefined or malformed. Consider adding validation:
| if (lastUserMessage && Array.isArray(lastUserMessage.content)) { | |
| if (lastUserMessage && Array.isArray(lastUserMessage.content)) { | |
| const textBlocks = lastUserMessage.content.filter((block: any) => block?.type === "text") | |
| if (textBlocks.length > 0) { | |
| // Combine all text blocks to form the user query | |
| userQuery = textBlocks.map((block: any) => block?.text || "").filter(Boolean).join("\n") | |
| } | |
| } |
| // Add dependencies | ||
| if (rule.dependencies) { | ||
| for (const depFilename of rule.dependencies) { | ||
| const depRule = availableRules.find((r) => r.filename === depFilename) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we optimize this dependency resolution? Currently it's O(n) for each dependency. Consider creating a Map at the start of the function for O(1) lookups:
| const depRule = availableRules.find((r) => r.filename === depFilename) | |
| const depRule = availableRules.find((r) => r.filename === depFilename) |
Alternatively, create const rulesMap = new Map(availableRules.map(r => [r.filename, r])) at the beginning and use rulesMap.get(depFilename).
| .trim() | ||
|
|
||
| try { | ||
| const frontmatter = yaml.parse(frontmatterContent) as SmartRuleFile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we validate the YAML structure here? The parsed frontmatter might not match our expected schema. Consider adding validation:
if (!frontmatter["use-when"] || typeof frontmatter["use-when"] !== "string") {
logger.warn("Invalid frontmatter: missing or invalid use-when field", { filePath })
return null
}| import { EXPERIMENT_IDS } from "./experiments" | ||
| import { TOOL_GROUPS, ALWAYS_AVAILABLE_TOOLS } from "./tools" | ||
|
|
||
| // Conditional import to avoid bundling Node.js modules in webview |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This conditional import pattern is clever! Could we add a comment explaining why it's necessary? Something like:
// Conditional import to avoid bundling Node.js modules in webview
// The webview environment doesn't have access to Node.js fs/path modules,
// so we dynamically import only when running in the extension context| }, | ||
| "roo-cline.smartRules.minSimilarity": { | ||
| "type": "number", | ||
| "default": 0.7, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is 0.7 too high as a default? Users might miss relevant rules. Consider lowering to 0.5 or 0.6 to be more inclusive by default, letting users increase it if they get too many matches.
|
Closing, needs approval first |
Summary
This PR implements the Intelligent Semantic Rule Injection feature as proposed in issue #6707. The feature addresses the current limitation where all applicable rules are injected into every chat session regardless of relevance, causing token waste, context pollution, and poor scalability.
What's Changed
Core Implementation
SmartRuleinterface and related types to define rule metadata structure.roo/smart-rules/directories with YAML frontmatter parsingFeatures
use-whenmetadata for context description.roo/smart-rules-{mode}/)Configuration Options
roo-cline.smartRules.enabled: Enable/disable smart rules (default: true)roo-cline.smartRules.minSimilarity: Minimum similarity score (default: 0.1)roo-cline.smartRules.maxRules: Maximum rules to inject (default: 10)roo-cline.smartRules.showSelectedRules: Show selected rules in outputroo-cline.smartRules.debugRuleSelection: Enable debug loggingTesting
Benefits
Example Smart Rule
Fixes #6707
Important
Implement intelligent semantic rule injection to optimize context-aware rule selection using Jaccard similarity and configurable settings.
SmartRuleinterface and types intypes/smart-rules.ts..roo/smart-rules/insmart-rules-loader.ts.smart-rules-matcher.ts.custom-instructions.ts.use-whenmetadata for context description.package.jsonfor enabling smart rules, similarity threshold, max rules, and debug options.smart-rules-loader.spec.ts.smart-rules-matcher.spec.ts.This description was created by
for 000dcda. You can customize this summary. It will automatically update as commits are pushed.