refactor(tool): use go-git for gitignore matching#141
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughReplaces the custom gitignore-rule engine in Changesgo-git gitignore integration
Sequence Diagram(s)No additional sequence diagrams are generated beyond those in the hidden artifact, as the core flow is already visualized there. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/tool/ignore.go (1)
95-98: 💤 Low valuePerformance consideration:
gitignore.ReadPatternstraverses the entire worktree.The
ReadPatternsfunction walks the directory tree to collect all.gitignorefiles recursively. For large repositories with deep directory structures, this could introduce latency on every read operation. Consider caching the patterns per workspace root if this becomes a bottleneck.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/tool/ignore.go` around lines 95 - 98, The gitignore.ReadPatterns function traverses the entire directory tree to collect patterns, which causes performance degradation on large repositories when called repeatedly. Implement a caching mechanism that stores the parsed patterns indexed by workspaceRoot to avoid repeated directory traversals. Before calling gitignore.ReadPatterns with osfs.New(workspaceRoot) and nil, check if patterns have already been cached for this workspaceRoot and return the cached result if available; only call ReadPatterns if the cache misses, and store the result in the cache before returning it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/tool/ignore.go`:
- Around line 95-98: The gitignore.ReadPatterns function traverses the entire
directory tree to collect patterns, which causes performance degradation on
large repositories when called repeatedly. Implement a caching mechanism that
stores the parsed patterns indexed by workspaceRoot to avoid repeated directory
traversals. Before calling gitignore.ReadPatterns with osfs.New(workspaceRoot)
and nil, check if patterns have already been cached for this workspaceRoot and
return the cached result if available; only call ReadPatterns if the cache
misses, and store the result in the cache before returning it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1f657d61-899f-4742-bcc3-becdf23e9ecf
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
go.modinternal/tool/ignore.gointernal/tool/read_test.go
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #141 +/- ##
==========================================
+ Coverage 80.36% 80.39% +0.02%
==========================================
Files 277 277
Lines 21824 21838 +14
==========================================
+ Hits 17539 17556 +17
+ Misses 3078 3074 -4
- Partials 1207 1208 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/tool/read.go`:
- Around line 29-30: The NewReadTool function creates a long-lived
readIgnoreCache that stores gitignore patterns, and these cached patterns are
reused without checking if the .gitignore file has been modified at runtime.
When .gitignore is edited during execution, the stale cached patterns in
readIgnoreCache.repositoryPatterns will not reflect the changes, causing files
to be misclassified. Add staleness detection by tracking the modification time
or hash of the .gitignore file, then validate the cache before using it at lines
76-77 by comparing the current .gitignore state against the previously cached
state and invalidating the cache if changes are detected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8e6afbbf-9ee2-4087-ba53-f98484f26970
📒 Files selected for processing (3)
internal/tool/ast.gointernal/tool/ignore.gointernal/tool/read.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/tool/ignore.go
|



No description provided.