feat(task-runner): add task filtering by tags and names#250
feat(task-runner): add task filtering by tags and names#250thalesraymond wants to merge 1 commit intomainfrom
Conversation
This commit implements the `add-task-filtering` specification from OpenSpec. It introduces the `TaskFilterConfig` interface, adds `tags` array to `TaskStep`, and provides a highly efficient `filterTasks` utility that respects inclusion, exclusion, and automatic dependency resolution rules. The configuration is wired into `TaskRunner.execute` to filter steps prior to execution and validation. Corresponding unit and integration tests have been added to ensure correctness and adherence to the specification. The OpenSpec change proposal has been successfully archived. 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! |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements task filtering within the task runner, allowing users to include or exclude tasks based on names and tags. It introduces the TaskFilterConfig interface, adds a tags field to TaskStep, and provides a filterTasks utility that handles complex filtering logic, including dependency resolution. The TaskRunner was updated to integrate this filtering into its execution flow. Feedback was provided to optimize the filterTasks implementation by consolidating multiple array iterations into a single pass for better performance.
| const { | ||
| includeTags, | ||
| excludeTags, | ||
| includeNames, | ||
| excludeNames, | ||
| includeDependencies = true, | ||
| } = config; | ||
|
|
||
| // Helper sets for quick lookup | ||
| const inclTagsSet = includeTags ? new Set(includeTags) : null; | ||
| const exclTagsSet = excludeTags ? new Set(excludeTags) : null; | ||
| const inclNamesSet = includeNames ? new Set(includeNames) : null; | ||
| const exclNamesSet = excludeNames ? new Set(excludeNames) : null; | ||
|
|
||
| // Map of task names to task objects for quick dependency lookup | ||
| const taskMap = new Map<string, TaskStep<TContext>>(); | ||
| for (let i = 0; i < steps.length; i++) { | ||
| taskMap.set(steps[i].name, steps[i]); | ||
| } | ||
|
|
||
| // 1. Determine initially included tasks | ||
| const includedTasks = new Set<string>(); | ||
|
|
||
| for (let i = 0; i < steps.length; i++) { | ||
| const task = steps[i]; | ||
| let isIncluded = false; | ||
|
|
||
| // If no inclusion criteria provided, include all initially | ||
| if (!inclTagsSet && !inclNamesSet) { | ||
| isIncluded = true; | ||
| } else { | ||
| // Check if it matches includeNames | ||
| if (inclNamesSet && inclNamesSet.has(task.name)) { | ||
| isIncluded = true; | ||
| } | ||
| // Check if it matches includeTags | ||
| if (!isIncluded && inclTagsSet && task.tags) { | ||
| for (let j = 0; j < task.tags.length; j++) { | ||
| if (inclTagsSet.has(task.tags[j])) { | ||
| isIncluded = true; | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (isIncluded) { | ||
| includedTasks.add(task.name); | ||
| } | ||
| } | ||
|
|
||
| // 2. Remove explicitly excluded tasks | ||
| const excludedTasks = new Set<string>(); | ||
| for (let i = 0; i < steps.length; i++) { | ||
| const task = steps[i]; | ||
| let isExcluded = false; | ||
|
|
||
| if (exclNamesSet && exclNamesSet.has(task.name)) { | ||
| isExcluded = true; | ||
| } | ||
|
|
||
| if (!isExcluded && exclTagsSet && task.tags) { | ||
| for (let j = 0; j < task.tags.length; j++) { | ||
| if (exclTagsSet.has(task.tags[j])) { | ||
| isExcluded = true; | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (isExcluded) { | ||
| excludedTasks.add(task.name); | ||
| includedTasks.delete(task.name); | ||
| } | ||
| } | ||
|
|
||
| // 3. Resolve dependencies if needed | ||
| if (includeDependencies) { | ||
| const queue = Array.from(includedTasks); | ||
| let queueIdx = 0; | ||
|
|
||
| while (queueIdx < queue.length) { | ||
| const taskName = queue[queueIdx++]; | ||
| const task = taskMap.get(taskName); | ||
|
|
||
| if (task && task.dependencies) { | ||
| for (let i = 0; i < task.dependencies.length; i++) { | ||
| const depName = task.dependencies[i]; | ||
|
|
||
| // If the dependency isn't already included and isn't explicitly excluded | ||
| if (!includedTasks.has(depName) && !excludedTasks.has(depName)) { | ||
| includedTasks.add(depName); | ||
| queue.push(depName); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // 4. Return the filtered tasks in the original order | ||
| const filteredSteps: TaskStep<TContext>[] = []; | ||
| for (let i = 0; i < steps.length; i++) { | ||
| if (includedTasks.has(steps[i].name)) { | ||
| filteredSteps.push(steps[i]); | ||
| } | ||
| } | ||
|
|
||
| return filteredSteps; |
There was a problem hiding this comment.
The current implementation of filterTasks iterates over the steps array multiple times. Consolidating these passes and using a Map for task lookups ensures O(1) access time, which is more efficient for frequent lookups. However, before merging loops for code simplification, consider the performance impact. Retaining separate logic can be justified if it avoids expensive operations and provides significant, benchmarked performance gains.
const {
includeTags,
excludeTags,
includeNames,
excludeNames,
includeDependencies = true,
} = config;
const inclTagsSet = includeTags ? new Set(includeTags) : null;
const exclTagsSet = excludeTags ? new Set(excludeTags) : null;
const inclNamesSet = includeNames ? new Set(includeNames) : null;
const exclNamesSet = excludeNames ? new Set(excludeNames) : null;
const taskMap = new Map<string, TaskStep<TContext>>();
const includedTasks = new Set<string>();
const excludedTasks = new Set<string>();
for (const task of steps) {
taskMap.set(task.name, task);
const matchesInclusion = (!inclTagsSet && !inclNamesSet) ||
(inclNamesSet?.has(task.name)) ||
(inclTagsSet && task.tags?.some(tag => inclTagsSet.has(tag)));
const matchesExclusion = (exclNamesSet?.has(task.name)) ||
(exclTagsSet && task.tags?.some(tag => exclTagsSet.has(tag)));
if (matchesExclusion) {
excludedTasks.add(task.name);
} else if (matchesInclusion) {
includedTasks.add(task.name);
}
}
if (includeDependencies) {
const queue = Array.from(includedTasks);
let queueIdx = 0;
while (queueIdx < queue.length) {
const taskName = queue[queueIdx++];
const task = taskMap.get(taskName);
if (task?.dependencies) {
for (const depName of task.dependencies) {
if (!includedTasks.has(depName) && !excludedTasks.has(depName)) {
includedTasks.add(depName);
queue.push(depName);
}
}
}
}
}
return steps.filter((task) => includedTasks.has(task.name));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.
- Before merging loops or removing duplicate checks for code simplification, consider the performance impact. Retaining separate logic can be justified if it avoids expensive operations and provides significant, benchmarked performance gains.
1 participant
This PR addresses the user's request to implement, verify, and archive the pending OpenSpec change proposal for adding "task filtering" functionality to the
task-runnerrepository.Key changes include:
tagsarray to theTaskStepinterface.TaskFilterConfiginterface to define inclusion/exclusion rules.filterTasksutility using sets and breadth-first search for efficient dependency inclusion and explicit exclusion evaluation.TaskRunner.executeto seamlessly apply the filter configuration before graph validation and execution.tests/utils/TaskFilter.test.ts) and integration tests (tests/TaskRunnerFiltering.test.ts).openspec/specs/task-runner/spec.mdand archiving the completed proposal.PR created automatically by Jules for task 2023531427021183161 started by @thalesraymond