feat: implement task filtering by tags and names#246
feat: implement task filtering by tags and names#246thalesraymond wants to merge 1 commit intomainfrom
Conversation
This commit fully implements the `add-task-filtering` specification. - Updated `TaskStep` to include an optional `tags` string array. - Added `TaskFilterConfig` for configuring `includeTags`, `excludeTags`, `includeNames`, `excludeNames`, and `includeDependencies`. - Created `filterTasks` utility to correctly resolve inclusive and exclusive combinations while optionally resolving dependency closures. - Configured `TaskRunner` to conditionally apply `filterTasks` to input steps before executing or validating the workflow logic. - Added comprehensive unit and integration tests. - Archived the spec manually by moving it to `openspec/changes/archive/`. Co-authored-by: thalesraymond <32554150+thalesraymond@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Code Review
This pull request introduces task filtering capabilities to the TaskRunner, allowing users to include or exclude tasks based on names and tags. It includes a new TaskFilterConfig interface, updates to TaskStep to support tags, and a filterTasks utility for processing these criteria. Feedback was provided to optimize the filterTasks implementation by using Set objects for O(1) lookups instead of repeated array iterations, which improves performance for large task sets.
| const stepMap = new Map<string, TaskStep<TContext>>(); | ||
| for (const step of steps) { | ||
| stepMap.set(step.name, step); | ||
| } | ||
|
|
||
| const initialMatches = new Set<string>(); | ||
|
|
||
| for (const step of steps) { | ||
| let included = true; | ||
|
|
||
| // Check inclusion criteria | ||
| if (!config.includeNames?.length && !config.includeTags?.length) { | ||
| included = true; | ||
| } else if (config.includeNames?.length && config.includeTags?.length) { | ||
| // If both are provided, we should match either. | ||
| const nameMatch = config.includeNames.includes(step.name); | ||
| const tagMatch = !!step.tags && step.tags.some(tag => config.includeTags!.includes(tag)); | ||
|
|
||
| included = nameMatch || tagMatch; | ||
| } else if (config.includeNames?.length) { | ||
| included = config.includeNames.includes(step.name); | ||
| } else { | ||
| // Only includeTags is provided | ||
| included = !!step.tags && step.tags.some(tag => config.includeTags!.includes(tag)); | ||
| } | ||
|
|
||
| // Check exclusion criteria | ||
| if (config.excludeNames && config.excludeNames.includes(step.name)) { | ||
| included = false; | ||
| } | ||
|
|
||
| if (config.excludeTags && !!step.tags && step.tags.some(tag => config.excludeTags!.includes(tag))) { | ||
| included = false; | ||
| } | ||
|
|
||
| if (included) { | ||
| initialMatches.add(step.name); | ||
| } | ||
| } | ||
|
|
||
| // Handle dependencies | ||
| if (config.includeDependencies) { | ||
| const queue = Array.from(initialMatches); | ||
| const visited = new Set<string>(initialMatches); | ||
|
|
||
| while (queue.length > 0) { | ||
| const currentName = queue.shift()!; | ||
| const currentStep = stepMap.get(currentName); | ||
|
|
||
| if (currentStep && currentStep.dependencies) { | ||
| for (const dep of currentStep.dependencies) { | ||
| // If a dependency is excluded, we might not include it, but | ||
| // standard behavior is usually to override exclusion for explicit dependencies, | ||
| // or fail. Let's include it unless it's explicitly excluded. | ||
| // Wait, if it's explicitly excluded, it might fail the pipeline. | ||
| // For now, if we include dependencies, we'll just add them unless they are in exclusions. | ||
| let depExcluded = false; | ||
| const depStep = stepMap.get(dep); | ||
|
|
||
| if (config.excludeNames && config.excludeNames.includes(dep)) { | ||
| depExcluded = true; | ||
| } | ||
| if (depStep && config.excludeTags && depStep.tags && depStep.tags.some(tag => config.excludeTags!.includes(tag))) { | ||
| depExcluded = true; | ||
| } | ||
|
|
||
| if (!visited.has(dep) && !depExcluded) { | ||
| visited.add(dep); | ||
| queue.push(dep); | ||
| initialMatches.add(dep); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The current implementation uses multiple array lookups (.includes()) and iterations (.some()) inside loops, leading to O(N*M) complexity. Converting filter criteria to Set objects provides O(1) lookups, which is significantly more efficient for larger task sets. Additionally, the inclusion and dependency resolution logic can be simplified for better maintainability and performance by avoiding redundant checks and sets.
const includeNames = new Set(config.includeNames ?? []);
const includeTags = new Set(config.includeTags ?? []);
const excludeNames = new Set(config.excludeNames ?? []);
const excludeTags = new Set(config.excludeTags ?? []);
const stepMap = new Map(steps.map((step) => [step.name, step]));
const initialMatches = new Set<string>();
const hasIncludeCriteria = includeNames.size > 0 || includeTags.size > 0;
for (const step of steps) {
let included = !hasIncludeCriteria;
if (hasIncludeCriteria) {
const nameMatch = includeNames.has(step.name);
const tagMatch = step.tags?.some((tag) => includeTags.has(tag)) ?? false;
included = nameMatch || tagMatch;
}
if (included && (excludeNames.has(step.name) || (step.tags?.some((tag) => excludeTags.has(tag)) ?? false))) {
included = false;
}
if (included) {
initialMatches.add(step.name);
}
}
if (config.includeDependencies) {
const queue = Array.from(initialMatches);
while (queue.length > 0) {
const currentName = queue.shift()!;
const currentStep = stepMap.get(currentName);
if (currentStep?.dependencies) {
for (const dep of currentStep.dependencies) {
if (initialMatches.has(dep)) continue;
const depStep = stepMap.get(dep);
const isExcluded = excludeNames.has(dep) ||
(depStep?.tags?.some((tag) => excludeTags.has(tag)) ?? false);
if (!isExcluded) {
initialMatches.add(dep);
queue.push(dep);
}
}
}
}
}References
- When frequent lookups of items by a specific key are required, use a Map (or dictionary/hash table) to store the items, indexed by that key, to ensure O(1) access time.



Implemented the
add-task-filteringpending change specification.The changes involve introducing task tags and robust filtering mechanisms via
TaskFilterConfig. Users can dynamically include or exclude tasks by names or tags, and optionally resolve their dependent closures natively insideTaskRunner.Comprehensive test suites were added, ensuring precise validation behavior. Pre-commit hooks including linting, building, and 100% code coverage tests passed. The proposal folders were then archived into
openspec/changes/archive/.PR created automatically by Jules for task 3023652389587345650 started by @thalesraymond