feat: Honor .gitignore and .cursorignore during AI file sampling#10
feat: Honor .gitignore and .cursorignore during AI file sampling#10
Conversation
Add proper gitignore support to prevent template/scaffolding files (like BMAD method files) from being included in AI analysis. This fixes incorrect project categorization caused by including files that should be ignored. Changes: - Add pathspec dependency for gitignore pattern matching - Implement _load_ignore_patterns() method to parse .gitignore and .cursorignore - Filter all sampled files against ignore patterns before AI analysis - Fix missing imports (glob, re) in project_service.py Resolves issue where projects were incorrectly tagged based on template files that should have been ignored.
|
|
WalkthroughAdded pathspec dependency to support ignore pattern loading. Implemented Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/project_manager_cli/services/project_service.py (2)
42-71: Implementation looks correct.The method properly loads patterns from both ignore files and constructs a PathSpec using the correct 'gitwildmatch' pattern style for gitignore syntax. The defensive exception handling ensures the tool continues to function even if ignore files are unreadable.
Note: Static analysis flags broad exception catching (lines 56, 67), but this is acceptable here for robustness since we want to gracefully degrade if ignore files can't be read.
119-125: Consider adding logging to the exception handler.The ignore pattern check works correctly, but the broad exception handler silently skips any path resolution issues. While
relative_toshould normally succeed duringos.walk, adding a debug log would help diagnose unexpected failures.🔎 Suggested improvement:
# Check if file is ignored by .gitignore or .cursorignore try: relative_path = str(p.relative_to(base_dir)) if ignore_spec.match_file(relative_path): continue - except Exception: + except Exception as e: + self.logger.debug(f"Could not check ignore status for {p}: {e}") continue
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pyproject.toml(1 hunks)src/project_manager_cli/services/project_service.py(5 hunks)
🧰 Additional context used
🪛 Ruff (0.14.8)
src/project_manager_cli/services/project_service.py
56-56: Do not catch blind exception: Exception
(BLE001)
57-57: Use explicit conversion flag
Replace with conversion flag
(RUF010)
67-67: Do not catch blind exception: Exception
(BLE001)
68-68: Use explicit conversion flag
Replace with conversion flag
(RUF010)
124-125: try-except-continue detected, consider logging the exception
(S112)
124-124: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (4)
src/project_manager_cli/services/project_service.py (3)
3-11: LGTM! Missing imports fixed.The addition of
globandreimports fixes previously missing imports (used at lines 228, 232-233), and the newListandpathspecimports support the ignore pattern functionality.
81-82: LGTM!Loading ignore patterns once at the beginning of file sampling is the correct approach for efficiency and consistency.
97-100: LGTM!The README filtering correctly respects ignore patterns. Since the path is constructed from
base_dir / name, therelative_tocall will always succeed, so no additional exception handling is needed here.pyproject.toml (1)
26-26: No security concerns with pathspec version 0.11.0.No direct vulnerabilities have been found for pathspec in Snyk's vulnerability database, and the latest available version is 0.12.1. The
>=0.11.0constraint is appropriate and allows upgrades to newer secure releases.
Code Review: PR #10 - Honor .gitignore and .cursorignore during AI file samplingOverviewExcellent PR! This addresses a real problem where template/scaffolding files were polluting AI analysis and causing incorrect project categorization. The implementation is solid and follows best practices. ✅ Strengths
🔍 Code Quality ObservationsArchitecture & Design
Error HandlingLine 124-125: The bare except (ValueError, OSError) as e:
self.logger.debug(f"Could not compute relative path for {p}: {e}")
continuePerformance Considerations
🐛 Potential Issues1. Empty Pattern HandlingWhen both .gitignore and .cursorignore are missing or empty, Suggestion: Add a debug log when no patterns are found: if not patterns:
self.logger.debug("No ignore patterns found in .gitignore or .cursorignore")2. Pattern FilteringThe code doesn't filter out comments ( Suggestion: Filter patterns when loading: patterns.extend(
line for line in f.read().splitlines()
if line.strip() and not line.strip().startswith('#')
)3. Cross-Platform Path HandlingOn Windows, pathspec expects forward slashes, but Verification needed: Test on Windows to ensure patterns match correctly. Potential fix: relative_path = str(p.relative_to(base_dir)).replace('\\', '/')🔒 Security Considerations
🧪 Test CoverageCurrent state: As noted in CLAUDE.md, this project has no formal test suite. Recommended test cases (when test infrastructure is added):
📝 DocumentationSuggestions:
🎯 RecommendationsBefore merging:
Future improvements:
🎖️ Final AssessmentQuality Score: 8.5/10 Recommendation: ✅ Approve with minor suggestions This PR successfully solves the stated problem and demonstrates good engineering practices. The implementation is clean, well-structured, and follows the project's conventions. The main concerns are Windows path handling (needs verification) and pattern parsing robustness (comments/blanks). These are minor issues that can be addressed in follow-up commits if needed. The addition of proper gitignore support is a significant quality improvement that will make AI tagging much more accurate and reliable. Great work! 🚀 |
Add proper gitignore support to prevent template/scaffolding files (like BMAD method files) from being included in AI analysis. This fixes incorrect project categorization caused by including files that should be ignored.
Changes:
Resolves issue where projects were incorrectly tagged based on template files that should have been ignored.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.