Skip to content

feat: implement task filtering by tags and names#246

Closed
thalesraymond wants to merge 1 commit intomainfrom
feat-add-task-filtering-3023652389587345650
Closed

feat: implement task filtering by tags and names#246
thalesraymond wants to merge 1 commit intomainfrom
feat-add-task-filtering-3023652389587345650

Conversation

@thalesraymond
Copy link
Copy Markdown
Owner

Implemented the add-task-filtering pending 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 inside TaskRunner.

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

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>
@google-labs-jules
Copy link
Copy Markdown
Contributor

👋 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 3, 2026

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +15 to +89
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);
}
}
}
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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
  1. 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant