Skip to content

fix: replace substring matching with path-component matching in exclusions - #159

Merged
danielmeppiel merged 3 commits into
microsoft:mainfrom
sergio-sisternes-epam:fix/158-substring-exclusion-false-positives
Mar 4, 2026
Merged

fix: replace substring matching with path-component matching in exclusions#159
danielmeppiel merged 3 commits into
microsoft:mainfrom
sergio-sisternes-epam:fix/158-substring-exclusion-false-positives

Conversation

@sergio-sisternes-epam

Copy link
Copy Markdown
Collaborator

Description

Replace substring matching (ignore in str(current_path)) with path-component matching (part in (...) for part in relative_parts) in _analyze_project_structure(). The substring approach caused false-positive exclusions for directories whose names contain an exclusion token (e.g. rebuild/ matching build, redistribution/ matching dist, apm_modules_guide/ matching apm_modules).

Fixes #158

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Maintenance / refactor

Testing

  • Tested locally
  • All existing tests pass
  • Added tests for new functionality (if applicable)

New tests (TestSubstringExclusionFalsePositives, 3 cases):

  • test_directory_containing_exclusion_token_not_excluded — verifies rebuild/, apm_modules_guide/, redistribution/, node_modules_compat/ are not excluded
  • test_exact_exclusion_names_still_excluded — verifies build/, dist/, node_modules/, __pycache__/ are still excluded
  • test_nested_exclusion_name_still_excluded — verifies src/build/ is still excluded

All 40 tests pass.

…sions

Replace `ignore in str(current_path)` with `part in (...) for part in
relative_parts` in _analyze_project_structure() to prevent false positive
exclusions. Directories like 'rebuild/', 'redistribution/', or
'apm_modules_guide/' were incorrectly excluded because their names
contain exclusion tokens ('build', 'dist', 'apm_modules') as substrings.

Fixes microsoft#158
Copilot AI review requested due to automatic review settings March 4, 2026 17:10
@sergio-sisternes-epam sergio-sisternes-epam added the bug Deprecated: use type/bug. Kept for issue history; will be removed in milestone 0.10.0. label Mar 4, 2026
@sergio-sisternes-epam sergio-sisternes-epam added this to the 0.8.0 milestone Mar 4, 2026

Copilot AI left a comment

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.

Pull request overview

Fixes incorrect directory exclusion behavior in ContextOptimizer._analyze_project_structure() by switching from substring-based checks to exact path-component matching, preventing false-positive exclusions like rebuild/ being excluded due to build.

Changes:

  • Replace substring matching on str(current_path) with path-component matching on current_path.relative_to(base_dir).parts for default exclusions.
  • Add unit tests covering false-positive cases (e.g., rebuild/, redistribution/, node_modules_compat/) and ensuring exact-name exclusions still apply.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/apm_cli/compilation/context_optimizer.py Updates default exclusion logic in _analyze_project_structure() to use path-component matching.
tests/unit/compilation/test_context_optimizer.py Adds regression tests to ensure substring false-positives are not excluded while exact matches still are.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread src/apm_cli/compilation/context_optimizer.py Outdated
Comment thread src/apm_cli/compilation/context_optimizer.py Outdated
Comment thread src/apm_cli/compilation/context_optimizer.py Outdated
- Reuse already-computed relative_path instead of calling relative_to()
  a second time, avoiding unguarded ValueError (review comment 1)
- Extract DEFAULT_EXCLUDED_DIRNAMES frozenset constant shared by
  _analyze_project_structure and _should_exclude_subdir (review comment 2)
- Update docs/cli-reference.md and docs/compilation.md to clarify that
  exclusions match on exact path components (review comment 3)
@danielmeppiel
danielmeppiel merged commit 65008d8 into microsoft:main Mar 4, 2026
6 checks passed
sergio-sisternes-epam added a commit that referenced this pull request May 19, 2026
- Reuse already-computed relative_path instead of calling relative_to()
  a second time, avoiding unguarded ValueError (review comment 1)
- Extract DEFAULT_EXCLUDED_DIRNAMES frozenset constant shared by
  _analyze_project_structure and _should_exclude_subdir (review comment 2)
- Update docs/cli-reference.md and docs/compilation.md to clarify that
  exclusions match on exact path components (review comment 3)
sergio-sisternes-epam pushed a commit that referenced this pull request May 19, 2026
…exclusion-false-positives

fix: replace substring matching with path-component matching in exclusions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Deprecated: use type/bug. Kept for issue history; will be removed in milestone 0.10.0.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Substring matching in default directory exclusions causes false positives

3 participants